The existing pxa driver and the upcoming addition of PWM support in the
TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
thereby a need for a of_xlate function with the period as its single
argument.
Introduce a common helper function in the core that can be used as
of_xlate by such drivers and migrate the pxa driver to use this.
Signed-off-by: Bjorn Andersson <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
Tested-by: Steev Klimaszewski <[email protected]>
---
Changes since v6:
- None
drivers/pwm/core.c | 26 ++++++++++++++++++++++++++
drivers/pwm/pwm-pxa.c | 16 +---------------
include/linux/pwm.h | 2 ++
3 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 4527f09a5c50..2c6b155002a2 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -152,6 +152,32 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
}
EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags);
+struct pwm_device *
+of_pwm_single_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
+{
+ struct pwm_device *pwm;
+
+ if (pc->of_pwm_n_cells < 1)
+ return ERR_PTR(-EINVAL);
+
+ /* validate that one cell is specified, optionally with flags */
+ if (args->args_count != 1 && args->args_count != 2)
+ return ERR_PTR(-EINVAL);
+
+ pwm = pwm_request_from_chip(pc, 0, NULL);
+ if (IS_ERR(pwm))
+ return pwm;
+
+ pwm->args.period = args->args[0];
+ pwm->args.polarity = PWM_POLARITY_NORMAL;
+
+ if (args->args_count == 2 && args->args[2] & PWM_POLARITY_INVERTED)
+ pwm->args.polarity = PWM_POLARITY_INVERSED;
+
+ return pwm;
+}
+EXPORT_SYMBOL_GPL(of_pwm_single_xlate);
+
static void of_pwmchip_add(struct pwm_chip *chip)
{
if (!chip->dev || !chip->dev->of_node)
diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
index a9efdcf839ae..238ec88c130b 100644
--- a/drivers/pwm/pwm-pxa.c
+++ b/drivers/pwm/pwm-pxa.c
@@ -148,20 +148,6 @@ static const struct platform_device_id *pxa_pwm_get_id_dt(struct device *dev)
return id ? id->data : NULL;
}
-static struct pwm_device *
-pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
-{
- struct pwm_device *pwm;
-
- pwm = pwm_request_from_chip(pc, 0, NULL);
- if (IS_ERR(pwm))
- return pwm;
-
- pwm->args.period = args->args[0];
-
- return pwm;
-}
-
static int pwm_probe(struct platform_device *pdev)
{
const struct platform_device_id *id = platform_get_device_id(pdev);
@@ -187,7 +173,7 @@ static int pwm_probe(struct platform_device *pdev)
pc->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 : 1;
if (IS_ENABLED(CONFIG_OF)) {
- pc->chip.of_xlate = pxa_pwm_of_xlate;
+ pc->chip.of_xlate = of_pwm_single_xlate;
pc->chip.of_pwm_n_cells = 1;
}
diff --git a/include/linux/pwm.h b/include/linux/pwm.h
index 725c9b784e60..dd51d4931fdc 100644
--- a/include/linux/pwm.h
+++ b/include/linux/pwm.h
@@ -414,6 +414,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
const struct of_phandle_args *args);
+struct pwm_device *of_pwm_single_xlate(struct pwm_chip *pc,
+ const struct of_phandle_args *args);
struct pwm_device *pwm_get(struct device *dev, const char *con_id);
struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
--
2.29.2
The multi-register u16 write operation can use regmap_bulk_write()
instead of two separate regmap_write() calls.
It's uncertain if this has any effect on the actual updates of the
underlying registers, but this at least gives the hardware the
opportunity and saves us one transation on the bus.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v6:
- None
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 6154bed0af5b..5b59d8dd3acd 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -193,8 +193,9 @@ static const struct regmap_config ti_sn65dsi86_regmap_config = {
static void ti_sn65dsi86_write_u16(struct ti_sn65dsi86 *pdata,
unsigned int reg, u16 val)
{
- regmap_write(pdata->regmap, reg, val & 0xFF);
- regmap_write(pdata->regmap, reg + 1, val >> 8);
+ u8 buf[2] = { val & 0xff, val >> 8 };
+
+ regmap_bulk_write(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
}
static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn65dsi86 *pdata)
--
2.29.2
On 10/25/21 12:09 PM, Bjorn Andersson wrote:
> The existing pxa driver and the upcoming addition of PWM support in the
> TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> thereby a need for a of_xlate function with the period as its single
> argument.
>
> Introduce a common helper function in the core that can be used as
> of_xlate by such drivers and migrate the pxa driver to use this.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> Acked-by: Uwe Kleine-König <[email protected]>
> Tested-by: Steev Klimaszewski <[email protected]>
> ---
>
> Changes since v6:
> - None
>
> drivers/pwm/core.c | 26 ++++++++++++++++++++++++++
> drivers/pwm/pwm-pxa.c | 16 +---------------
> include/linux/pwm.h | 2 ++
> 3 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 4527f09a5c50..2c6b155002a2 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -152,6 +152,32 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
> }
> EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags);
>
> +struct pwm_device *
> +of_pwm_single_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> +{
> + struct pwm_device *pwm;
> +
> + if (pc->of_pwm_n_cells < 1)
> + return ERR_PTR(-EINVAL);
> +
> + /* validate that one cell is specified, optionally with flags */
> + if (args->args_count != 1 && args->args_count != 2)
> + return ERR_PTR(-EINVAL);
> +
> + pwm = pwm_request_from_chip(pc, 0, NULL);
> + if (IS_ERR(pwm))
> + return pwm;
> +
> + pwm->args.period = args->args[0];
> + pwm->args.polarity = PWM_POLARITY_NORMAL;
> +
> + if (args->args_count == 2 && args->args[2] & PWM_POLARITY_INVERTED)
> + pwm->args.polarity = PWM_POLARITY_INVERSED;
> +
> + return pwm;
> +}
> +EXPORT_SYMBOL_GPL(of_pwm_single_xlate);
> +
> static void of_pwmchip_add(struct pwm_chip *chip)
> {
> if (!chip->dev || !chip->dev->of_node)
> diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
> index a9efdcf839ae..238ec88c130b 100644
> --- a/drivers/pwm/pwm-pxa.c
> +++ b/drivers/pwm/pwm-pxa.c
> @@ -148,20 +148,6 @@ static const struct platform_device_id *pxa_pwm_get_id_dt(struct device *dev)
> return id ? id->data : NULL;
> }
>
> -static struct pwm_device *
> -pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> -{
> - struct pwm_device *pwm;
> -
> - pwm = pwm_request_from_chip(pc, 0, NULL);
> - if (IS_ERR(pwm))
> - return pwm;
> -
> - pwm->args.period = args->args[0];
> -
> - return pwm;
> -}
> -
> static int pwm_probe(struct platform_device *pdev)
> {
> const struct platform_device_id *id = platform_get_device_id(pdev);
> @@ -187,7 +173,7 @@ static int pwm_probe(struct platform_device *pdev)
> pc->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 : 1;
>
> if (IS_ENABLED(CONFIG_OF)) {
> - pc->chip.of_xlate = pxa_pwm_of_xlate;
> + pc->chip.of_xlate = of_pwm_single_xlate;
> pc->chip.of_pwm_n_cells = 1;
> }
>
> diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> index 725c9b784e60..dd51d4931fdc 100644
> --- a/include/linux/pwm.h
> +++ b/include/linux/pwm.h
> @@ -414,6 +414,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
>
> struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
> const struct of_phandle_args *args);
> +struct pwm_device *of_pwm_single_xlate(struct pwm_chip *pc,
> + const struct of_phandle_args *args);
>
> struct pwm_device *pwm_get(struct device *dev, const char *con_id);
> struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
v7 of the series is tested by me on the Lenovo Yoga C630
Tested-By: Steev Klimaszewski <[email protected]>
On Wed 27 Oct 01:29 PDT 2021, Robert Foss wrote:
> Hey Bjorn,
>
> On Mon, 25 Oct 2021 at 19:07, Bjorn Andersson
> <[email protected]> wrote:
> >
> > The multi-register u16 write operation can use regmap_bulk_write()
> > instead of two separate regmap_write() calls.
> >
> > It's uncertain if this has any effect on the actual updates of the
> > underlying registers, but this at least gives the hardware the
> > opportunity and saves us one transation on the bus.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
>
> Did you miss including Dougs R-B from v6? As far as I can tell nothing
> else changed between v6 & v7.
>
Yes, I missed adding Doug's R-b from v6. I also missed fixing the
spelling of transaction (transation) in the commit message.
Would you be willing to correct these two items as you apply the
patches?
Thanks,
Bjorn
> > ---
> >
> > Changes since v6:
> > - None
> >
> > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index 6154bed0af5b..5b59d8dd3acd 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -193,8 +193,9 @@ static const struct regmap_config ti_sn65dsi86_regmap_config = {
> > static void ti_sn65dsi86_write_u16(struct ti_sn65dsi86 *pdata,
> > unsigned int reg, u16 val)
> > {
> > - regmap_write(pdata->regmap, reg, val & 0xFF);
> > - regmap_write(pdata->regmap, reg + 1, val >> 8);
> > + u8 buf[2] = { val & 0xff, val >> 8 };
> > +
> > + regmap_bulk_write(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
> > }
> >
> > static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn65dsi86 *pdata)
> > --
> > 2.29.2
> >
On Tue, 26 Oct 2021 at 19:21, Steev Klimaszewski <[email protected]> wrote:
>
>
> On 10/25/21 12:09 PM, Bjorn Andersson wrote:
> > The existing pxa driver and the upcoming addition of PWM support in the
> > TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> > thereby a need for a of_xlate function with the period as its single
> > argument.
> >
> > Introduce a common helper function in the core that can be used as
> > of_xlate by such drivers and migrate the pxa driver to use this.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > Acked-by: Uwe Kleine-König <[email protected]>
> > Tested-by: Steev Klimaszewski <[email protected]>
> > ---
> >
> > Changes since v6:
> > - None
> >
> > drivers/pwm/core.c | 26 ++++++++++++++++++++++++++
> > drivers/pwm/pwm-pxa.c | 16 +---------------
> > include/linux/pwm.h | 2 ++
> > 3 files changed, 29 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> > index 4527f09a5c50..2c6b155002a2 100644
> > --- a/drivers/pwm/core.c
> > +++ b/drivers/pwm/core.c
> > @@ -152,6 +152,32 @@ of_pwm_xlate_with_flags(struct pwm_chip *pc, const struct of_phandle_args *args)
> > }
> > EXPORT_SYMBOL_GPL(of_pwm_xlate_with_flags);
> >
> > +struct pwm_device *
> > +of_pwm_single_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> > +{
> > + struct pwm_device *pwm;
> > +
> > + if (pc->of_pwm_n_cells < 1)
> > + return ERR_PTR(-EINVAL);
> > +
> > + /* validate that one cell is specified, optionally with flags */
> > + if (args->args_count != 1 && args->args_count != 2)
> > + return ERR_PTR(-EINVAL);
> > +
> > + pwm = pwm_request_from_chip(pc, 0, NULL);
> > + if (IS_ERR(pwm))
> > + return pwm;
> > +
> > + pwm->args.period = args->args[0];
> > + pwm->args.polarity = PWM_POLARITY_NORMAL;
> > +
> > + if (args->args_count == 2 && args->args[2] & PWM_POLARITY_INVERTED)
> > + pwm->args.polarity = PWM_POLARITY_INVERSED;
> > +
> > + return pwm;
> > +}
> > +EXPORT_SYMBOL_GPL(of_pwm_single_xlate);
> > +
> > static void of_pwmchip_add(struct pwm_chip *chip)
> > {
> > if (!chip->dev || !chip->dev->of_node)
> > diff --git a/drivers/pwm/pwm-pxa.c b/drivers/pwm/pwm-pxa.c
> > index a9efdcf839ae..238ec88c130b 100644
> > --- a/drivers/pwm/pwm-pxa.c
> > +++ b/drivers/pwm/pwm-pxa.c
> > @@ -148,20 +148,6 @@ static const struct platform_device_id *pxa_pwm_get_id_dt(struct device *dev)
> > return id ? id->data : NULL;
> > }
> >
> > -static struct pwm_device *
> > -pxa_pwm_of_xlate(struct pwm_chip *pc, const struct of_phandle_args *args)
> > -{
> > - struct pwm_device *pwm;
> > -
> > - pwm = pwm_request_from_chip(pc, 0, NULL);
> > - if (IS_ERR(pwm))
> > - return pwm;
> > -
> > - pwm->args.period = args->args[0];
> > -
> > - return pwm;
> > -}
> > -
> > static int pwm_probe(struct platform_device *pdev)
> > {
> > const struct platform_device_id *id = platform_get_device_id(pdev);
> > @@ -187,7 +173,7 @@ static int pwm_probe(struct platform_device *pdev)
> > pc->chip.npwm = (id->driver_data & HAS_SECONDARY_PWM) ? 2 : 1;
> >
> > if (IS_ENABLED(CONFIG_OF)) {
> > - pc->chip.of_xlate = pxa_pwm_of_xlate;
> > + pc->chip.of_xlate = of_pwm_single_xlate;
> > pc->chip.of_pwm_n_cells = 1;
> > }
> >
> > diff --git a/include/linux/pwm.h b/include/linux/pwm.h
> > index 725c9b784e60..dd51d4931fdc 100644
> > --- a/include/linux/pwm.h
> > +++ b/include/linux/pwm.h
> > @@ -414,6 +414,8 @@ struct pwm_device *pwm_request_from_chip(struct pwm_chip *chip,
> >
> > struct pwm_device *of_pwm_xlate_with_flags(struct pwm_chip *pc,
> > const struct of_phandle_args *args);
> > +struct pwm_device *of_pwm_single_xlate(struct pwm_chip *pc,
> > + const struct of_phandle_args *args);
> >
> > struct pwm_device *pwm_get(struct device *dev, const char *con_id);
> > struct pwm_device *of_pwm_get(struct device *dev, struct device_node *np,
>
> v7 of the series is tested by me on the Lenovo Yoga C630
>
> Tested-By: Steev Klimaszewski <[email protected]>
>
Applied to drm-misc-next.
On Wed, 27 Oct 2021 at 16:04, Bjorn Andersson
<[email protected]> wrote:
>
> On Wed 27 Oct 01:29 PDT 2021, Robert Foss wrote:
>
> > Hey Bjorn,
> >
> > On Mon, 25 Oct 2021 at 19:07, Bjorn Andersson
> > <[email protected]> wrote:
> > >
> > > The multi-register u16 write operation can use regmap_bulk_write()
> > > instead of two separate regmap_write() calls.
> > >
> > > It's uncertain if this has any effect on the actual updates of the
> > > underlying registers, but this at least gives the hardware the
> > > opportunity and saves us one transation on the bus.
> > >
> > > Signed-off-by: Bjorn Andersson <[email protected]>
> >
> > Did you miss including Dougs R-B from v6? As far as I can tell nothing
> > else changed between v6 & v7.
> >
>
> Yes, I missed adding Doug's R-b from v6. I also missed fixing the
> spelling of transaction (transation) in the commit message.
>
> Would you be willing to correct these two items as you apply the
> patches?
Can do.
Hey Bjorn,
On Mon, 25 Oct 2021 at 19:07, Bjorn Andersson
<[email protected]> wrote:
>
> The multi-register u16 write operation can use regmap_bulk_write()
> instead of two separate regmap_write() calls.
>
> It's uncertain if this has any effect on the actual updates of the
> underlying registers, but this at least gives the hardware the
> opportunity and saves us one transation on the bus.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
Did you miss including Dougs R-B from v6? As far as I can tell nothing
else changed between v6 & v7.
> ---
>
> Changes since v6:
> - None
>
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 6154bed0af5b..5b59d8dd3acd 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -193,8 +193,9 @@ static const struct regmap_config ti_sn65dsi86_regmap_config = {
> static void ti_sn65dsi86_write_u16(struct ti_sn65dsi86 *pdata,
> unsigned int reg, u16 val)
> {
> - regmap_write(pdata->regmap, reg, val & 0xFF);
> - regmap_write(pdata->regmap, reg + 1, val >> 8);
> + u8 buf[2] = { val & 0xff, val >> 8 };
> +
> + regmap_bulk_write(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
> }
>
> static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn65dsi86 *pdata)
> --
> 2.29.2
>
Hello,
On Wed, Oct 27, 2021 at 05:06:02PM +0200, Robert Foss wrote:
> On Tue, 26 Oct 2021 at 19:21, Steev Klimaszewski <[email protected]> wrote:
> >
> >
> > On 10/25/21 12:09 PM, Bjorn Andersson wrote:
> > > The existing pxa driver and the upcoming addition of PWM support in the
> > > TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> > > thereby a need for a of_xlate function with the period as its single
> > > argument.
> > >
> > > Introduce a common helper function in the core that can be used as
> > > of_xlate by such drivers and migrate the pxa driver to use this.
> > >
> > > Signed-off-by: Bjorn Andersson <[email protected]>
> > > Acked-by: Uwe Kleine-K?nig <[email protected]>
> > > Tested-by: Steev Klimaszewski <[email protected]>
> > > ---
> > >
> [...]
>
> Applied to drm-misc-next.
This is now 3ab7b6ac5d829e60c3b89d415811ff1c9f358c8e in next, the Link:
added in the commit trailer looks as follows:
Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
but this link doesn't work, for me at least. I wonder what's wrong with
it. If you want to fix it and rewrite the commit, you can also drop the
duplicated "Tested-by: Steev Klimaszewski <[email protected]>".
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hey Uwe
On Sat, 30 Oct 2021 at 11:27, Uwe Kleine-König
<[email protected]> wrote:
>
> Hello,
>
> On Wed, Oct 27, 2021 at 05:06:02PM +0200, Robert Foss wrote:
> > On Tue, 26 Oct 2021 at 19:21, Steev Klimaszewski <[email protected]> wrote:
> > >
> > >
> > > On 10/25/21 12:09 PM, Bjorn Andersson wrote:
> > > > The existing pxa driver and the upcoming addition of PWM support in the
> > > > TI sn565dsi86 DSI/eDP bridge driver both has a single PWM channel and
> > > > thereby a need for a of_xlate function with the period as its single
> > > > argument.
> > > >
> > > > Introduce a common helper function in the core that can be used as
> > > > of_xlate by such drivers and migrate the pxa driver to use this.
> > > >
> > > > Signed-off-by: Bjorn Andersson <[email protected]>
> > > > Acked-by: Uwe Kleine-König <[email protected]>
> > > > Tested-by: Steev Klimaszewski <[email protected]>
> > > > ---
> > > >
> > [...]
> >
> > Applied to drm-misc-next.
>
> This is now 3ab7b6ac5d829e60c3b89d415811ff1c9f358c8e in next, the Link:
> added in the commit trailer looks as follows:
>
> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
>
> but this link doesn't work, for me at least. I wonder what's wrong with
> it. If you want to fix it and rewrite the commit, you can also drop the
> duplicated "Tested-by: Steev Klimaszewski <[email protected]>".
Weirdly patchwork.fd.o[1] doesn't seem to have the series, but does
have previous versions.
[1] https://patchwork.freedesktop.org/project/dri-devel/patches/?submitter=&state=*&q=_xlate&archive=both&delegate=