This series implements 16 channel PWM support for Intel Elkhart Lake
along with minor cleanups for DesignWare PWM driver.
Changes since v2:
- Remove error code duplication from dev_err_probe()
- Update tags
Changes since v1:
- Drop redundant error check
- Provide dwc_pwm_init_one() to initialize one PWM instance
- Use dev_get_drvdata() instead of pci_get_drvdata()
- Use pm_sleep_ptr() instead of use pm_ptr()
Raag Jadav (5):
pwm: dwc: use pm_sleep_ptr() macro
pwm: dwc: drop redundant error check
pwm: dwc: Add 16 channel support for Intel Elkhart Lake
pwm: dwc: simplify error handling
pwm: dwc: access driver_data using dev_get_drvdata()
drivers/pwm/pwm-dwc.c | 59 ++++++++++++++++++++++++-------------------
drivers/pwm/pwm-dwc.h | 5 ++++
2 files changed, 38 insertions(+), 26 deletions(-)
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
--
2.35.3
Now that we're setting driver_data using dev_set_drvdata(), we can use
dev_get_drvdata() for accessing it.
Signed-off-by: Raag Jadav <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/pwm/pwm-dwc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 56fac8655c7b..ed56b796b670 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -82,8 +82,7 @@ static void dwc_pwm_remove(struct pci_dev *pci)
static int dwc_pwm_suspend(struct device *dev)
{
- struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
- struct dwc_pwm *dwc = pci_get_drvdata(pdev);
+ struct dwc_pwm *dwc = dev_get_drvdata(dev);
int i;
for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
@@ -102,8 +101,7 @@ static int dwc_pwm_suspend(struct device *dev)
static int dwc_pwm_resume(struct device *dev)
{
- struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
- struct dwc_pwm *dwc = pci_get_drvdata(pdev);
+ struct dwc_pwm *dwc = dev_get_drvdata(dev);
int i;
for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
--
2.35.3
Intel Elkhart Lake PSE includes two instances of PWM as a single PCI
function with 8 channels each. Add support for the remaining channels.
Signed-off-by: Raag Jadav <[email protected]>
Tested-by: Lakshmi Sowjanya D <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/pwm/pwm-dwc.c | 33 +++++++++++++++++++++++++--------
drivers/pwm/pwm-dwc.h | 5 +++++
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index b9e18dbf7493..6d922afcb20a 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -25,16 +25,31 @@
#include "pwm-dwc.h"
-static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
+/* Elkhart Lake */
+static const struct dwc_pwm_info ehl_pwm_info = {
+ .nr = 2,
+ .size = 0x1000,
+};
+
+static int dwc_pwm_init_one(struct device *dev, void __iomem *base, unsigned int offset)
{
- struct device *dev = &pci->dev;
struct dwc_pwm *dwc;
- int ret;
dwc = dwc_pwm_alloc(dev);
if (!dwc)
return -ENOMEM;
+ dwc->base = base + offset;
+
+ return devm_pwmchip_add(dev, &dwc->chip);
+}
+
+static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
+{
+ const struct dwc_pwm_info *info;
+ struct device *dev = &pci->dev;
+ int i, ret;
+
ret = pcim_enable_device(pci);
if (ret) {
dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret));
@@ -49,11 +64,13 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
return ret;
}
- dwc->base = pcim_iomap_table(pci)[0];
+ info = (const struct dwc_pwm_info *)id->driver_data;
- ret = devm_pwmchip_add(dev, &dwc->chip);
- if (ret)
- return ret;
+ for (i = 0; i < info->nr; i++) {
+ ret = dwc_pwm_init_one(dev, pcim_iomap_table(pci)[0], i * info->size);
+ if (ret)
+ return ret;
+ }
pm_runtime_put(dev);
pm_runtime_allow(dev);
@@ -105,7 +122,7 @@ static int dwc_pwm_resume(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(dwc_pwm_pm_ops, dwc_pwm_suspend, dwc_pwm_resume);
static const struct pci_device_id dwc_pwm_id_table[] = {
- { PCI_VDEVICE(INTEL, 0x4bb7) }, /* Elkhart Lake */
+ { PCI_VDEVICE(INTEL, 0x4bb7), (kernel_ulong_t)&ehl_pwm_info },
{ } /* Terminating Entry */
};
MODULE_DEVICE_TABLE(pci, dwc_pwm_id_table);
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
index 64795247c54c..c9bbfc77b568 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -33,6 +33,11 @@ MODULE_IMPORT_NS(dwc_pwm);
#define DWC_TIM_CTRL_INT_MASK BIT(2)
#define DWC_TIM_CTRL_PWM BIT(3)
+struct dwc_pwm_info {
+ unsigned int nr;
+ unsigned int size;
+};
+
struct dwc_pwm_ctx {
u32 cnt;
u32 cnt2;
--
2.35.3
pcim_iomap_table() fails only if pcim_iomap_regions() fails. No need to
check for failure if the latter is already successful.
Suggested-by: Andy Shevchenko <[email protected]>
Signed-off-by: Raag Jadav <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/pwm/pwm-dwc.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index a4a057ae03ea..b9e18dbf7493 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -50,10 +50,6 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
}
dwc->base = pcim_iomap_table(pci)[0];
- if (!dwc->base) {
- dev_err(dev, "Base address missing\n");
- return -ENOMEM;
- }
ret = devm_pwmchip_add(dev, &dwc->chip);
if (ret)
--
2.35.3
Simplify error handling in ->probe() function using dev_err_probe() helper
and while at it, drop error codes from the message to prevent duplication.
Signed-off-by: Raag Jadav <[email protected]>
---
drivers/pwm/pwm-dwc.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 6d922afcb20a..56fac8655c7b 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -51,18 +51,14 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
int i, ret;
ret = pcim_enable_device(pci);
- if (ret) {
- dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret));
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to enable device\n");
pci_set_master(pci);
ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci));
- if (ret) {
- dev_err(dev, "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret));
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to iomap PCI BAR\n");
info = (const struct dwc_pwm_info *)id->driver_data;
--
2.35.3
Since we don't have runtime PM handles here, we should be using
pm_sleep_ptr() macro, so that the compiler can discard it in case
CONFIG_PM_SLEEP=n.
Fixes: 30b5b066fa83 ("pwm: dwc: Use DEFINE_SIMPLE_DEV_PM_OPS for PM functions")
Signed-off-by: Raag Jadav <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/pwm/pwm-dwc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 4929354f8cd9..a4a057ae03ea 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -120,7 +120,7 @@ static struct pci_driver dwc_pwm_driver = {
.remove = dwc_pwm_remove,
.id_table = dwc_pwm_id_table,
.driver = {
- .pm = pm_ptr(&dwc_pwm_pm_ops),
+ .pm = pm_sleep_ptr(&dwc_pwm_pm_ops),
},
};
--
2.35.3
On 2/12/24 15:02, Raag Jadav wrote:
> This series implements 16 channel PWM support for Intel Elkhart Lake
> along with minor cleanups for DesignWare PWM driver.
>
> Changes since v2:
> - Remove error code duplication from dev_err_probe()
> - Update tags
>
> Changes since v1:
> - Drop redundant error check
> - Provide dwc_pwm_init_one() to initialize one PWM instance
> - Use dev_get_drvdata() instead of pci_get_drvdata()
> - Use pm_sleep_ptr() instead of use pm_ptr()
>
> Raag Jadav (5):
> pwm: dwc: use pm_sleep_ptr() macro
> pwm: dwc: drop redundant error check
> pwm: dwc: Add 16 channel support for Intel Elkhart Lake
> pwm: dwc: simplify error handling
> pwm: dwc: access driver_data using dev_get_drvdata()
>
> drivers/pwm/pwm-dwc.c | 59 ++++++++++++++++++++++++-------------------
> drivers/pwm/pwm-dwc.h | 5 ++++
> 2 files changed, 38 insertions(+), 26 deletions(-)
>
I tested on Elkhart lake this patchset and it adds another 8 channel PWM
instance and the PCI device continue switching between D0 and D3 power
states depending is some channel enabled or all idle.
Tested-by: Jarkko Nikula <[email protected]>
Hello,
On Mon, Feb 12, 2024 at 06:32:44PM +0530, Raag Jadav wrote:
> pcim_iomap_table() fails only if pcim_iomap_regions() fails. No need to
> check for failure if the latter is already successful.
>
> Suggested-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Raag Jadav <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
> ---
> drivers/pwm/pwm-dwc.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> index a4a057ae03ea..b9e18dbf7493 100644
> --- a/drivers/pwm/pwm-dwc.c
> +++ b/drivers/pwm/pwm-dwc.c
> @@ -50,10 +50,6 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
> }
>
> dwc->base = pcim_iomap_table(pci)[0];
> - if (!dwc->base) {
> - dev_err(dev, "Base address missing\n");
> - return -ENOMEM;
> - }
As just written in reply to v2, I'd like to have a comment here saying
that pcim_iomap_table() won't fail after pcim_iomap_table() to prevent
someone sending a patch that undoes this change with the reasoning that
pcim_iomap_table()'s return value should be checked.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |