| Message ID | 20220906195735.87361-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1,1/9] pwm: lpss: Deduplicate board info data structures | expand |
On Tue, Sep 06, 2022 at 10:57:27PM +0300, Andy Shevchenko wrote: > With help of __maybe_unused, that allows to avoid compilation warnings, > move the board info structures from the C files to the common header > and hence deduplicate configuration data. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pwm/pwm-lpss-pci.c | 29 ----------------------------- > drivers/pwm/pwm-lpss-platform.c | 23 ----------------------- > drivers/pwm/pwm-lpss.h | 30 ++++++++++++++++++++++++++++++ Given that both the pci driver and the platform driver alread depend on pwm-lpss.o, I'd prefer something like the patch below to really deduplicate the data. One thing to note is that the two pwm_lpss_bsw_info are not identical. I didn't check how that is relevant. Did you check that? Best regards Uwe diff --git a/drivers/pwm/pwm-lpss-pci.c b/drivers/pwm/pwm-lpss-pci.c index c893ec3d2fb4..75b778e839b3 100644 --- a/drivers/pwm/pwm-lpss-pci.c +++ b/drivers/pwm/pwm-lpss-pci.c @@ -14,35 +14,6 @@ #include "pwm-lpss.h" -/* BayTrail */ -static const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { - .clk_rate = 25000000, - .npwm = 1, - .base_unit_bits = 16, -}; - -/* Braswell */ -static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { - .clk_rate = 19200000, - .npwm = 1, - .base_unit_bits = 16, -}; - -/* Broxton */ -static const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, - .bypass = true, -}; - -/* Tangier */ -static const struct pwm_lpss_boardinfo pwm_lpss_tng_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, -}; - static int pwm_lpss_probe_pci(struct pci_dev *pdev, const struct pci_device_id *id) { diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index 928570430cef..834423c34f48 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -15,28 +15,6 @@ #include "pwm-lpss.h" -/* BayTrail */ -static const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { - .clk_rate = 25000000, - .npwm = 1, - .base_unit_bits = 16, -}; - -/* Braswell */ -static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { - .clk_rate = 19200000, - .npwm = 1, - .base_unit_bits = 16, - .other_devices_aml_touches_pwm_regs = true, -}; - -/* Broxton */ -static const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, - .bypass = true, -}; static int pwm_lpss_probe_platform(struct platform_device *pdev) { diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c index 36d4e83e6b79..e8b16b67bfd4 100644 --- a/drivers/pwm/pwm-lpss.c +++ b/drivers/pwm/pwm-lpss.c @@ -29,6 +29,39 @@ /* Size of each PWM register space if multiple */ #define PWM_SIZE 0x400 +/* BayTrail */ +const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { + .clk_rate = 25000000, + .npwm = 1, + .base_unit_bits = 16, +}; +EXPORT_SYMBOL_GPL(pwm_lpss_byt_info); + +/* Braswell */ +const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { + .clk_rate = 19200000, + .npwm = 1, + .base_unit_bits = 16, +}; +EXPORT_SYMBOL_GPL(pwm_lpss_bsw_info); + +/* Broxton */ +const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { + .clk_rate = 19200000, + .npwm = 4, + .base_unit_bits = 22, + .bypass = true, +}; +EXPORT_SYMBOL_GPL(pwm_lpss_bxt_info); + +/* Tangier */ +const struct pwm_lpss_boardinfo pwm_lpss_tng_info = { + .clk_rate = 19200000, + .npwm = 4, + .base_unit_bits = 22, +}; +EXPORT_SYMBOL_GPL(pwm_lpss_tng_info); + static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip) { return container_of(chip, struct pwm_lpss_chip, chip); diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h index 8b3476f25e06..918d2f177109 100644 --- a/drivers/pwm/pwm-lpss.h +++ b/drivers/pwm/pwm-lpss.h @@ -33,6 +33,11 @@ struct pwm_lpss_boardinfo { bool other_devices_aml_touches_pwm_regs; }; +extern const struct pwm_lpss_boardinfo pwm_lpss_tng_info; +extern const struct pwm_lpss_boardinfo pwm_lpss_bxt_info; +extern const struct pwm_lpss_boardinfo pwm_lpss_bsw_info; +extern const struct pwm_lpss_boardinfo pwm_lpss_byt_info; + struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, const struct pwm_lpss_boardinfo *info);
On Wed, Sep 07, 2022 at 11:04:12AM +0200, Uwe Kleine-König wrote: > On Tue, Sep 06, 2022 at 10:57:27PM +0300, Andy Shevchenko wrote: > > With help of __maybe_unused, that allows to avoid compilation warnings, > > move the board info structures from the C files to the common header > > and hence deduplicate configuration data. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > --- > > drivers/pwm/pwm-lpss-pci.c | 29 ----------------------------- > > drivers/pwm/pwm-lpss-platform.c | 23 ----------------------- > > drivers/pwm/pwm-lpss.h | 30 ++++++++++++++++++++++++++++++ > > Given that both the pci driver and the platform driver alread depend on > pwm-lpss.o, I'd prefer something like the patch below to really > deduplicate the data. Why not? I can use yours in v2. Can I get your SoB tag? > One thing to note is that the two pwm_lpss_bsw_info are not identical. I > didn't check how that is relevant. Did you check that? Yes, ACPI version should be used. Because switch to ACPI/PCI is done in BIOS while quite likely the rest of AML code is the same, meaning similar issue might be observed. The no bug report is due to no PCI enabled device in the wild, I think, and only reference boards can be tested, so nobody really cares about PCI Braswell case.
On Wed, Sep 07, 2022 at 05:27:22PM +0300, Andy Shevchenko wrote: > On Wed, Sep 07, 2022 at 11:04:12AM +0200, Uwe Kleine-König wrote: > > On Tue, Sep 06, 2022 at 10:57:27PM +0300, Andy Shevchenko wrote: > > > With help of __maybe_unused, that allows to avoid compilation warnings, > > > move the board info structures from the C files to the common header > > > and hence deduplicate configuration data. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > --- > > > drivers/pwm/pwm-lpss-pci.c | 29 ----------------------------- > > > drivers/pwm/pwm-lpss-platform.c | 23 ----------------------- > > > drivers/pwm/pwm-lpss.h | 30 ++++++++++++++++++++++++++++++ > > > > Given that both the pci driver and the platform driver alread depend on > > pwm-lpss.o, I'd prefer something like the patch below to really > > deduplicate the data. > > Why not? I can use yours in v2. Can I get your SoB tag? Sure: Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > One thing to note is that the two pwm_lpss_bsw_info are not identical. I > > didn't check how that is relevant. Did you check that? > > Yes, ACPI version should be used. Because switch to ACPI/PCI is done in BIOS > while quite likely the rest of AML code is the same, meaning similar issue > might be observed. The no bug report is due to no PCI enabled device in the > wild, I think, and only reference boards can be tested, so nobody really cares > about PCI Braswell case. I'm willing to believe that; please mention that in the commit log. Best regards Uwe
diff --git a/drivers/pwm/pwm-lpss-pci.c b/drivers/pwm/pwm-lpss-pci.c index c893ec3d2fb4..75b778e839b3 100644 --- a/drivers/pwm/pwm-lpss-pci.c +++ b/drivers/pwm/pwm-lpss-pci.c @@ -14,35 +14,6 @@ #include "pwm-lpss.h" -/* BayTrail */ -static const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { - .clk_rate = 25000000, - .npwm = 1, - .base_unit_bits = 16, -}; - -/* Braswell */ -static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { - .clk_rate = 19200000, - .npwm = 1, - .base_unit_bits = 16, -}; - -/* Broxton */ -static const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, - .bypass = true, -}; - -/* Tangier */ -static const struct pwm_lpss_boardinfo pwm_lpss_tng_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, -}; - static int pwm_lpss_probe_pci(struct pci_dev *pdev, const struct pci_device_id *id) { diff --git a/drivers/pwm/pwm-lpss-platform.c b/drivers/pwm/pwm-lpss-platform.c index 928570430cef..fcd80cca2f6d 100644 --- a/drivers/pwm/pwm-lpss-platform.c +++ b/drivers/pwm/pwm-lpss-platform.c @@ -15,29 +15,6 @@ #include "pwm-lpss.h" -/* BayTrail */ -static const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { - .clk_rate = 25000000, - .npwm = 1, - .base_unit_bits = 16, -}; - -/* Braswell */ -static const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { - .clk_rate = 19200000, - .npwm = 1, - .base_unit_bits = 16, - .other_devices_aml_touches_pwm_regs = true, -}; - -/* Broxton */ -static const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { - .clk_rate = 19200000, - .npwm = 4, - .base_unit_bits = 22, - .bypass = true, -}; - static int pwm_lpss_probe_platform(struct platform_device *pdev) { const struct pwm_lpss_boardinfo *info; diff --git a/drivers/pwm/pwm-lpss.h b/drivers/pwm/pwm-lpss.h index 8b3476f25e06..3864f32c2487 100644 --- a/drivers/pwm/pwm-lpss.h +++ b/drivers/pwm/pwm-lpss.h @@ -33,6 +33,36 @@ struct pwm_lpss_boardinfo { bool other_devices_aml_touches_pwm_regs; }; +/* BayTrail */ +static __maybe_unused const struct pwm_lpss_boardinfo pwm_lpss_byt_info = { + .clk_rate = 25000000, + .npwm = 1, + .base_unit_bits = 16, +}; + +/* Braswell */ +static __maybe_unused const struct pwm_lpss_boardinfo pwm_lpss_bsw_info = { + .clk_rate = 19200000, + .npwm = 1, + .base_unit_bits = 16, + .other_devices_aml_touches_pwm_regs = true, +}; + +/* Broxton */ +static __maybe_unused const struct pwm_lpss_boardinfo pwm_lpss_bxt_info = { + .clk_rate = 19200000, + .npwm = 4, + .base_unit_bits = 22, + .bypass = true, +}; + +/* Tangier */ +static __maybe_unused const struct pwm_lpss_boardinfo pwm_lpss_tng_info = { + .clk_rate = 19200000, + .npwm = 4, + .base_unit_bits = 22, +}; + struct pwm_lpss_chip *pwm_lpss_probe(struct device *dev, struct resource *r, const struct pwm_lpss_boardinfo *info);
With help of __maybe_unused, that allows to avoid compilation warnings, move the board info structures from the C files to the common header and hence deduplicate configuration data. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pwm/pwm-lpss-pci.c | 29 ----------------------------- drivers/pwm/pwm-lpss-platform.c | 23 ----------------------- drivers/pwm/pwm-lpss.h | 30 ++++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 52 deletions(-)