| Message ID | 20221108142226.63161-7-andriy.shevchenko@linux.intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pinctrl: intel: Enable PWM optional feature | expand |
On Tue, Nov 08, 2022 at 04:22:26PM +0200, Andy Shevchenko wrote: > Some of the Communities may have PWM capability. In such cases, > enumerate PWM device via respective driver. User is still responsible > for setting correct pin muxing for the line that needs to output the > signal. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/pinctrl/intel/pinctrl-intel.c | 29 +++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c > index 6e630e87fed6..6b685ff7041f 100644 > --- a/drivers/pinctrl/intel/pinctrl-intel.c > +++ b/drivers/pinctrl/intel/pinctrl-intel.c > @@ -24,6 +24,8 @@ > #include <linux/pinctrl/pinctrl.h> > #include <linux/pinctrl/pinmux.h> > > +#include <linux/platform_data/x86/pwm-lpss.h> > + > #include "../core.h" > #include "pinctrl-intel.h" > > @@ -49,6 +51,8 @@ > #define PADOWN_MASK(p) (GENMASK(3, 0) << PADOWN_SHIFT(p)) > #define PADOWN_GPP(p) ((p) / 8) > > +#define PWMC 0x204 > + > /* Offset from pad_regs */ > #define PADCFG0 0x000 > #define PADCFG0_RXEVCFG_SHIFT 25 > @@ -1502,6 +1506,27 @@ static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl) > return 0; > } > > +static int intel_pinctrl_probe_pwm(struct intel_pinctrl *pctrl, > + struct intel_community *community) > +{ > + static const struct pwm_lpss_boardinfo info = { > + .clk_rate = 19200000, > + .npwm = 1, > + .base_unit_bits = 22, > + .bypass = true, > + }; > + struct pwm_lpss_chip *pwm; > + > + if (!(community->features & PINCTRL_FEATURE_PWM)) > + return 0; > + > + pwm = pwm_lpss_probe(pctrl->dev, community->regs + PWMC, &info); > + if (IS_ERR(pwm) && PTR_ERR(pwm) != -ENODEV) > + return PTR_ERR(pwm); Linus and Andy already agreed that this patch is ugly. I wonder if this here would be a bit less ugly if you do: if (IS_REACHABLE(...)) { pwm = pwm_lpss_probe(...); ... } and drop the check PTR_ERR(pwm) != -ENODEV (which might have a different semantic than "the pwm driver isn't available"). Best regards Uwe
On Thu, Nov 10, 2022 at 9:45 AM Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > On Tue, Nov 08, 2022 at 04:22:26PM +0200, Andy Shevchenko wrote: > > Some of the Communities may have PWM capability. In such cases, > > enumerate PWM device via respective driver. User is still responsible > > for setting correct pin muxing for the line that needs to output the > > signal. ... > > + pwm = pwm_lpss_probe(pctrl->dev, community->regs + PWMC, &info); > > + if (IS_ERR(pwm) && PTR_ERR(pwm) != -ENODEV) > > + return PTR_ERR(pwm); > > Linus and Andy already agreed that this patch is ugly. I wonder if this > here would be a bit less ugly if you do: > > if (IS_REACHABLE(...)) { > pwm = pwm_lpss_probe(...); > ... > > > } > > and drop the check PTR_ERR(pwm) != -ENODEV (which might have a different > semantic than "the pwm driver isn't available"). I will think about it (in such case the comment against the previous patch makes more sense to me). Thank you for the review!
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c index 6e630e87fed6..6b685ff7041f 100644 --- a/drivers/pinctrl/intel/pinctrl-intel.c +++ b/drivers/pinctrl/intel/pinctrl-intel.c @@ -24,6 +24,8 @@ #include <linux/pinctrl/pinctrl.h> #include <linux/pinctrl/pinmux.h> +#include <linux/platform_data/x86/pwm-lpss.h> + #include "../core.h" #include "pinctrl-intel.h" @@ -49,6 +51,8 @@ #define PADOWN_MASK(p) (GENMASK(3, 0) << PADOWN_SHIFT(p)) #define PADOWN_GPP(p) ((p) / 8) +#define PWMC 0x204 + /* Offset from pad_regs */ #define PADCFG0 0x000 #define PADCFG0_RXEVCFG_SHIFT 25 @@ -1502,6 +1506,27 @@ static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl) return 0; } +static int intel_pinctrl_probe_pwm(struct intel_pinctrl *pctrl, + struct intel_community *community) +{ + static const struct pwm_lpss_boardinfo info = { + .clk_rate = 19200000, + .npwm = 1, + .base_unit_bits = 22, + .bypass = true, + }; + struct pwm_lpss_chip *pwm; + + if (!(community->features & PINCTRL_FEATURE_PWM)) + return 0; + + pwm = pwm_lpss_probe(pctrl->dev, community->regs + PWMC, &info); + if (IS_ERR(pwm) && PTR_ERR(pwm) != -ENODEV) + return PTR_ERR(pwm); + + return 0; +} + static int intel_pinctrl_probe(struct platform_device *pdev, const struct intel_pinctrl_soc_data *soc_data) { @@ -1588,6 +1613,10 @@ static int intel_pinctrl_probe(struct platform_device *pdev, ret = intel_pinctrl_add_padgroups_by_size(pctrl, community); if (ret) return ret; + + ret = intel_pinctrl_probe_pwm(pctrl, community); + if (ret) + return ret; } irq = platform_get_irq(pdev, 0);