[PATCH v7 1/3] pwm: Introduce single-PWM of_xlate function

[PATCH v7 1/3] pwm: Introduce single-PWM of_xlate function

[Bjorn Andersson]

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 <bjorn.andersson@linaro.org>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Steev Klimaszewski <steev@kali.org>
---

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

[PATCH v7 2/3] drm/bridge: ti-sn65dsi86: Use regmap_bulk_write API

[Bjorn Andersson]

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 <bjorn.andersson@linaro.org>
---

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

Re: [PATCH v7 2/3] drm/bridge: ti-sn65dsi86: Use regmap_bulk_write API

[Robert Foss]

Hey Bjorn,

On Mon, 25 Oct 2021 at 19:07, Bjorn Andersson
<bjorn.andersson@linaro.org> 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 <bjorn.andersson@linaro.org>

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
>

Re: [PATCH v7 2/3] drm/bridge: ti-sn65dsi86: Use regmap_bulk_write API

[Bjorn Andersson]

On Wed 27 Oct 01:29 PDT 2021, Robert Foss wrote:

> Hey Bjorn,
> 
> On Mon, 25 Oct 2021 at 19:07, Bjorn Andersson
> <bjorn.andersson@linaro.org> 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 <bjorn.andersson@linaro.org>
> 
> 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
> >

Re: [PATCH v7 2/3] drm/bridge: ti-sn65dsi86: Use regmap_bulk_write API

[Robert Foss]

On Wed, 27 Oct 2021 at 16:04, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Wed 27 Oct 01:29 PDT 2021, Robert Foss wrote:
>
> > Hey Bjorn,
> >
> > On Mon, 25 Oct 2021 at 19:07, Bjorn Andersson
> > <bjorn.andersson@linaro.org> 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 <bjorn.andersson@linaro.org>
> >
> > 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.

[PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[Bjorn Andersson]

The SN65DSI86 provides the ability to supply a PWM signal on GPIO 4,
with the primary purpose of controlling the backlight of the attached
panel. Add an implementation that exposes this using the standard PWM
framework, to allow e.g. pwm-backlight to expose this to the user.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v6:
- Updated comment to clarify T_pwm's relationship to PWM_FREQ
- "period" was used unassigned when compared with period_max

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 364 +++++++++++++++++++++++++-
 1 file changed, 358 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 5b59d8dd3acd..430067a3071c 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -4,7 +4,9 @@
  * datasheet: https://www.ti.com/lit/ds/symlink/sn65dsi86.pdf
  */
 
+#include <linux/atomic.h>
 #include <linux/auxiliary_bus.h>
+#include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/clk.h>
 #include <linux/debugfs.h>
@@ -15,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/of_graph.h>
 #include <linux/pm_runtime.h>
+#include <linux/pwm.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
@@ -91,6 +94,13 @@
 #define SN_ML_TX_MODE_REG			0x96
 #define  ML_TX_MAIN_LINK_OFF			0
 #define  ML_TX_NORMAL_MODE			BIT(0)
+#define SN_PWM_PRE_DIV_REG			0xA0
+#define SN_BACKLIGHT_SCALE_REG			0xA1
+#define  BACKLIGHT_SCALE_MAX			0xFFFF
+#define SN_BACKLIGHT_REG			0xA3
+#define SN_PWM_EN_INV_REG			0xA5
+#define  SN_PWM_INV_MASK			BIT(0)
+#define  SN_PWM_EN_MASK				BIT(1)
 #define SN_AUX_CMD_STATUS_REG			0xF4
 #define  AUX_IRQ_STATUS_AUX_RPLY_TOUT		BIT(3)
 #define  AUX_IRQ_STATUS_AUX_SHORT		BIT(5)
@@ -113,11 +123,14 @@
 
 #define SN_LINK_TRAINING_TRIES		10
 
+#define SN_PWM_GPIO_IDX			3 /* 4th GPIO */
+
 /**
  * struct ti_sn65dsi86 - Platform data for ti-sn65dsi86 driver.
  * @bridge_aux:   AUX-bus sub device for MIPI-to-eDP bridge functionality.
  * @gpio_aux:     AUX-bus sub device for GPIO controller functionality.
  * @aux_aux:      AUX-bus sub device for eDP AUX channel functionality.
+ * @pwm_aux:      AUX-bus sub device for PWM controller functionality.
  *
  * @dev:          Pointer to the top level (i2c) device.
  * @regmap:       Regmap for accessing i2c.
@@ -145,11 +158,17 @@
  *                bitmap so we can do atomic ops on it without an extra
  *                lock so concurrent users of our 4 GPIOs don't stomp on
  *                each other's read-modify-write.
+ *
+ * @pchip:        pwm_chip if the PWM is exposed.
+ * @pwm_enabled:  Used to track if the PWM signal is currently enabled.
+ * @pwm_pin_busy: Track if GPIO4 is currently requested for GPIO or PWM.
+ * @pwm_refclk_freq: Cache for the reference clock input to the PWM.
  */
 struct ti_sn65dsi86 {
 	struct auxiliary_device		bridge_aux;
 	struct auxiliary_device		gpio_aux;
 	struct auxiliary_device		aux_aux;
+	struct auxiliary_device		pwm_aux;
 
 	struct device			*dev;
 	struct regmap			*regmap;
@@ -172,6 +191,12 @@ struct ti_sn65dsi86 {
 	struct gpio_chip		gchip;
 	DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
 #endif
+#if defined(CONFIG_PWM)
+	struct pwm_chip			pchip;
+	bool				pwm_enabled;
+	atomic_t			pwm_pin_busy;
+#endif
+	unsigned int			pwm_refclk_freq;
 };
 
 static const struct regmap_range ti_sn65dsi86_volatile_ranges[] = {
@@ -190,6 +215,21 @@ static const struct regmap_config ti_sn65dsi86_regmap_config = {
 	.cache_type = REGCACHE_NONE,
 };
 
+static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
+				 unsigned int reg, u16 *val)
+{
+	u8 buf[2];
+	int ret;
+
+	ret = regmap_bulk_read(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
+	if (ret)
+		return ret;
+
+	*val = buf[0] | (buf[1] << 8);
+
+	return 0;
+}
+
 static void ti_sn65dsi86_write_u16(struct ti_sn65dsi86 *pdata,
 				   unsigned int reg, u16 val)
 {
@@ -254,6 +294,12 @@ static void ti_sn_bridge_set_refclk_freq(struct ti_sn65dsi86 *pdata)
 
 	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
 			   REFCLK_FREQ(i));
+
+	/*
+	 * The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
+	 * regardless of its actual sourcing.
+	 */
+	pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
 }
 
 static void ti_sn65dsi86_enable_comms(struct ti_sn65dsi86 *pdata)
@@ -1261,9 +1307,287 @@ static struct auxiliary_driver ti_sn_bridge_driver = {
 };
 
 /* -----------------------------------------------------------------------------
- * GPIO Controller
+ * PWM Controller
  */
+#if defined(CONFIG_PWM)
+static int ti_sn_pwm_pin_request(struct ti_sn65dsi86 *pdata)
+{
+	return atomic_xchg(&pdata->pwm_pin_busy, 1) ? -EBUSY : 0;
+}
+
+static void ti_sn_pwm_pin_release(struct ti_sn65dsi86 *pdata)
+{
+	atomic_set(&pdata->pwm_pin_busy, 0);
+}
+
+static struct ti_sn65dsi86 *pwm_chip_to_ti_sn_bridge(struct pwm_chip *chip)
+{
+	return container_of(chip, struct ti_sn65dsi86, pchip);
+}
+
+static int ti_sn_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct ti_sn65dsi86 *pdata = pwm_chip_to_ti_sn_bridge(chip);
 
+	return ti_sn_pwm_pin_request(pdata);
+}
+
+static void ti_sn_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct ti_sn65dsi86 *pdata = pwm_chip_to_ti_sn_bridge(chip);
+
+	ti_sn_pwm_pin_release(pdata);
+}
+
+/*
+ * Limitations:
+ * - The PWM signal is not driven when the chip is powered down, or in its
+ *   reset state and the driver does not implement the "suspend state"
+ *   described in the documentation. In order to save power, state->enabled is
+ *   interpreted as denoting if the signal is expected to be valid, and is used
+ *   to determine if the chip needs to be kept powered.
+ * - Changing both period and duty_cycle is not done atomically, neither is the
+ *   multi-byte register updates, so the output might briefly be undefined
+ *   during update.
+ */
+static int ti_sn_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+			   const struct pwm_state *state)
+{
+	struct ti_sn65dsi86 *pdata = pwm_chip_to_ti_sn_bridge(chip);
+	unsigned int pwm_en_inv;
+	unsigned int backlight;
+	unsigned int pre_div;
+	unsigned int scale;
+	u64 period_max;
+	u64 period;
+	int ret;
+
+	if (!pdata->pwm_enabled) {
+		ret = pm_runtime_get_sync(pdata->dev);
+		if (ret < 0) {
+			pm_runtime_put_sync(pdata->dev);
+			return ret;
+		}
+	}
+
+	if (state->enabled) {
+		if (!pdata->pwm_enabled) {
+			/*
+			 * The chip might have been powered down while we
+			 * didn't hold a PM runtime reference, so mux in the
+			 * PWM function on the GPIO pin again.
+			 */
+			ret = regmap_update_bits(pdata->regmap, SN_GPIO_CTRL_REG,
+						 SN_GPIO_MUX_MASK << (2 * SN_PWM_GPIO_IDX),
+						 SN_GPIO_MUX_SPECIAL << (2 * SN_PWM_GPIO_IDX));
+			if (ret) {
+				dev_err(pdata->dev, "failed to mux in PWM function\n");
+				goto out;
+			}
+		}
+
+		/*
+		 * Per the datasheet the PWM frequency is given by:
+		 *
+		 *                          REFCLK_FREQ
+		 *   PWM_FREQ = -----------------------------------
+		 *               PWM_PRE_DIV * BACKLIGHT_SCALE + 1
+		 *
+		 * However, after careful review the author is convinced that
+		 * the documentation has lost some parenthesis around
+		 * "BACKLIGHT_SCALE + 1".
+		 *
+		 * With the period T_pwm = 1/PWM_FREQ this can be written:
+		 *
+		 *   T_pwm * REFCLK_FREQ = PWM_PRE_DIV * (BACKLIGHT_SCALE + 1)
+		 *
+		 * In order to keep BACKLIGHT_SCALE within its 16 bits,
+		 * PWM_PRE_DIV must be:
+		 *
+		 *                     T_pwm * REFCLK_FREQ
+		 *   PWM_PRE_DIV >= -------------------------
+		 *                   BACKLIGHT_SCALE_MAX + 1
+		 *
+		 * To simplify the search and to favour higher resolution of
+		 * the duty cycle over accuracy of the period, the lowest
+		 * possible PWM_PRE_DIV is used. Finally the scale is
+		 * calculated as:
+		 *
+		 *                      T_pwm * REFCLK_FREQ
+		 *   BACKLIGHT_SCALE = ---------------------- - 1
+		 *                          PWM_PRE_DIV
+		 *
+		 * Here T_pwm is represented in seconds, so appropriate scaling
+		 * to nanoseconds is necessary.
+		 */
+
+		/* Minimum T_pwm is 1 / REFCLK_FREQ */
+		if (state->period <= NSEC_PER_SEC / pdata->pwm_refclk_freq) {
+			ret = -EINVAL;
+			goto out;
+		}
+
+		/*
+		 * Maximum T_pwm is 255 * (65535 + 1) / REFCLK_FREQ
+		 * Limit period to this to avoid overflows
+		 */
+		period_max = div_u64((u64)NSEC_PER_SEC * 255 * (65535 + 1),
+				     pdata->pwm_refclk_freq);
+		period = min(state->period, period_max);
+
+		pre_div = DIV64_U64_ROUND_UP(period * pdata->pwm_refclk_freq,
+					     (u64)NSEC_PER_SEC * (BACKLIGHT_SCALE_MAX + 1));
+		scale = div64_u64(period * pdata->pwm_refclk_freq, (u64)NSEC_PER_SEC * pre_div) - 1;
+
+		/*
+		 * The documentation has the duty ratio given as:
+		 *
+		 *     duty          BACKLIGHT
+		 *   ------- = ---------------------
+		 *    period    BACKLIGHT_SCALE + 1
+		 *
+		 * Solve for BACKLIGHT, substituting BACKLIGHT_SCALE according
+		 * to definition above and adjusting for nanosecond
+		 * representation of duty cycle gives us:
+		 */
+		backlight = div64_u64(state->duty_cycle * pdata->pwm_refclk_freq,
+				      (u64)NSEC_PER_SEC * pre_div);
+		if (backlight > scale)
+			backlight = scale;
+
+		ret = regmap_write(pdata->regmap, SN_PWM_PRE_DIV_REG, pre_div);
+		if (ret) {
+			dev_err(pdata->dev, "failed to update PWM_PRE_DIV\n");
+			goto out;
+		}
+
+		ti_sn65dsi86_write_u16(pdata, SN_BACKLIGHT_SCALE_REG, scale);
+		ti_sn65dsi86_write_u16(pdata, SN_BACKLIGHT_REG, backlight);
+	}
+
+	pwm_en_inv = FIELD_PREP(SN_PWM_EN_MASK, state->enabled) |
+		     FIELD_PREP(SN_PWM_INV_MASK, state->polarity == PWM_POLARITY_INVERSED);
+	ret = regmap_write(pdata->regmap, SN_PWM_EN_INV_REG, pwm_en_inv);
+	if (ret) {
+		dev_err(pdata->dev, "failed to update PWM_EN/PWM_INV\n");
+		goto out;
+	}
+
+	pdata->pwm_enabled = state->enabled;
+out:
+
+	if (!pdata->pwm_enabled)
+		pm_runtime_put_sync(pdata->dev);
+
+	return ret;
+}
+
+static void ti_sn_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+				struct pwm_state *state)
+{
+	struct ti_sn65dsi86 *pdata = pwm_chip_to_ti_sn_bridge(chip);
+	unsigned int pwm_en_inv;
+	unsigned int pre_div;
+	u16 backlight;
+	u16 scale;
+	int ret;
+
+	ret = regmap_read(pdata->regmap, SN_PWM_EN_INV_REG, &pwm_en_inv);
+	if (ret)
+		return;
+
+	ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_SCALE_REG, &scale);
+	if (ret)
+		return;
+
+	ret = ti_sn65dsi86_read_u16(pdata, SN_BACKLIGHT_REG, &backlight);
+	if (ret)
+		return;
+
+	ret = regmap_read(pdata->regmap, SN_PWM_PRE_DIV_REG, &pre_div);
+	if (ret)
+		return;
+
+	state->enabled = FIELD_GET(SN_PWM_EN_MASK, pwm_en_inv);
+	if (FIELD_GET(SN_PWM_INV_MASK, pwm_en_inv))
+		state->polarity = PWM_POLARITY_INVERSED;
+	else
+		state->polarity = PWM_POLARITY_NORMAL;
+
+	state->period = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * pre_div * (scale + 1),
+					 pdata->pwm_refclk_freq);
+	state->duty_cycle = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * pre_div * backlight,
+					     pdata->pwm_refclk_freq);
+
+	if (state->duty_cycle > state->period)
+		state->duty_cycle = state->period;
+}
+
+static const struct pwm_ops ti_sn_pwm_ops = {
+	.request = ti_sn_pwm_request,
+	.free = ti_sn_pwm_free,
+	.apply = ti_sn_pwm_apply,
+	.get_state = ti_sn_pwm_get_state,
+	.owner = THIS_MODULE,
+};
+
+static int ti_sn_pwm_probe(struct auxiliary_device *adev,
+			   const struct auxiliary_device_id *id)
+{
+	struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
+
+	pdata->pchip.dev = pdata->dev;
+	pdata->pchip.ops = &ti_sn_pwm_ops;
+	pdata->pchip.npwm = 1;
+	pdata->pchip.of_xlate = of_pwm_single_xlate;
+	pdata->pchip.of_pwm_n_cells = 1;
+
+	return pwmchip_add(&pdata->pchip);
+}
+
+static void ti_sn_pwm_remove(struct auxiliary_device *adev)
+{
+	struct ti_sn65dsi86 *pdata = dev_get_drvdata(adev->dev.parent);
+
+	pwmchip_remove(&pdata->pchip);
+
+	if (pdata->pwm_enabled)
+		pm_runtime_put_sync(pdata->dev);
+}
+
+static const struct auxiliary_device_id ti_sn_pwm_id_table[] = {
+	{ .name = "ti_sn65dsi86.pwm", },
+	{},
+};
+
+static struct auxiliary_driver ti_sn_pwm_driver = {
+	.name = "pwm",
+	.probe = ti_sn_pwm_probe,
+	.remove = ti_sn_pwm_remove,
+	.id_table = ti_sn_pwm_id_table,
+};
+
+static int __init ti_sn_pwm_register(void)
+{
+	return auxiliary_driver_register(&ti_sn_pwm_driver);
+}
+
+static void ti_sn_pwm_unregister(void)
+{
+	auxiliary_driver_unregister(&ti_sn_pwm_driver);
+}
+
+#else
+static inline int ti_sn_pwm_pin_request(struct ti_sn65dsi86 *pdata) { return 0; }
+static inline void ti_sn_pwm_pin_release(struct ti_sn65dsi86 *pdata) {}
+
+static inline int ti_sn_pwm_register(void) { return 0; }
+static inline void ti_sn_pwm_unregister(void) {}
+#endif
+
+/* -----------------------------------------------------------------------------
+ * GPIO Controller
+ */
 #if defined(CONFIG_OF_GPIO)
 
 static int tn_sn_bridge_of_xlate(struct gpio_chip *chip,
@@ -1396,10 +1720,25 @@ static int ti_sn_bridge_gpio_direction_output(struct gpio_chip *chip,
 	return ret;
 }
 
+static int ti_sn_bridge_gpio_request(struct gpio_chip *chip, unsigned int offset)
+{
+	struct ti_sn65dsi86 *pdata = gpiochip_get_data(chip);
+
+	if (offset == SN_PWM_GPIO_IDX)
+		return ti_sn_pwm_pin_request(pdata);
+
+	return 0;
+}
+
 static void ti_sn_bridge_gpio_free(struct gpio_chip *chip, unsigned int offset)
 {
+	struct ti_sn65dsi86 *pdata = gpiochip_get_data(chip);
+
 	/* We won't keep pm_runtime if we're input, so switch there on free */
 	ti_sn_bridge_gpio_direction_input(chip, offset);
+
+	if (offset == SN_PWM_GPIO_IDX)
+		ti_sn_pwm_pin_release(pdata);
 }
 
 static const char * const ti_sn_bridge_gpio_names[SN_NUM_GPIOS] = {
@@ -1421,6 +1760,7 @@ static int ti_sn_gpio_probe(struct auxiliary_device *adev,
 	pdata->gchip.owner = THIS_MODULE;
 	pdata->gchip.of_xlate = tn_sn_bridge_of_xlate;
 	pdata->gchip.of_gpio_n_cells = 2;
+	pdata->gchip.request = ti_sn_bridge_gpio_request;
 	pdata->gchip.free = ti_sn_bridge_gpio_free;
 	pdata->gchip.get_direction = ti_sn_bridge_gpio_get_direction;
 	pdata->gchip.direction_input = ti_sn_bridge_gpio_direction_input;
@@ -1547,10 +1887,9 @@ static int ti_sn65dsi86_probe(struct i2c_client *client,
 	 * ordering. The bridge wants the panel to be there when it probes.
 	 * The panel wants its HPD GPIO (provided by sn65dsi86 on some boards)
 	 * when it probes. The panel and maybe backlight might want the DDC
-	 * bus. Soon the PWM provided by the bridge chip will have the same
-	 * problem. Having sub-devices allows the some sub devices to finish
-	 * probing even if others return -EPROBE_DEFER and gets us around the
-	 * problems.
+	 * bus or the pwm_chip. Having sub-devices allows the some sub devices
+	 * to finish probing even if others return -EPROBE_DEFER and gets us
+	 * around the problems.
 	 */
 
 	if (IS_ENABLED(CONFIG_OF_GPIO)) {
@@ -1559,6 +1898,12 @@ static int ti_sn65dsi86_probe(struct i2c_client *client,
 			return ret;
 	}
 
+	if (IS_ENABLED(CONFIG_PWM)) {
+		ret = ti_sn65dsi86_add_aux_device(pdata, &pdata->pwm_aux, "pwm");
+		if (ret)
+			return ret;
+	}
+
 	/*
 	 * NOTE: At the end of the AUX channel probe we'll add the aux device
 	 * for the bridge. This is because the bridge can't be used until the
@@ -1602,10 +1947,14 @@ static int __init ti_sn65dsi86_init(void)
 	if (ret)
 		goto err_main_was_registered;
 
-	ret = auxiliary_driver_register(&ti_sn_aux_driver);
+	ret = ti_sn_pwm_register();
 	if (ret)
 		goto err_gpio_was_registered;
 
+	ret = auxiliary_driver_register(&ti_sn_aux_driver);
+	if (ret)
+		goto err_pwm_was_registered;
+
 	ret = auxiliary_driver_register(&ti_sn_bridge_driver);
 	if (ret)
 		goto err_aux_was_registered;
@@ -1614,6 +1963,8 @@ static int __init ti_sn65dsi86_init(void)
 
 err_aux_was_registered:
 	auxiliary_driver_unregister(&ti_sn_aux_driver);
+err_pwm_was_registered:
+	ti_sn_pwm_unregister();
 err_gpio_was_registered:
 	ti_sn_gpio_unregister();
 err_main_was_registered:
@@ -1627,6 +1978,7 @@ static void __exit ti_sn65dsi86_exit(void)
 {
 	auxiliary_driver_unregister(&ti_sn_bridge_driver);
 	auxiliary_driver_unregister(&ti_sn_aux_driver);
+	ti_sn_pwm_unregister();
 	ti_sn_gpio_unregister();
 	i2c_del_driver(&ti_sn65dsi86_driver);
 }
-- 
2.29.2

Re: [PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

Hello,

On Mon, Oct 25, 2021 at 10:09:25AM -0700, Bjorn Andersson wrote:
> The SN65DSI86 provides the ability to supply a PWM signal on GPIO 4,
> with the primary purpose of controlling the backlight of the attached
> panel. Add an implementation that exposes this using the standard PWM
> framework, to allow e.g. pwm-backlight to expose this to the user.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2363 bytes --]

Hi Bjorn,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.15-rc6]
[also build test ERROR on next-20211026]
[cannot apply to thierry-reding-pwm/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
base:    519d81956ee277b4419c723adfb154603c2565ba
config: openrisc-buildonly-randconfig-r001-20211027 (attached as .config)
compiler: or1k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/e5ac651e6c7c724f3cd33e0fe322e714c0796348
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
        git checkout e5ac651e6c7c724f3cd33e0fe322e714c0796348
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=openrisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/ti-sn65dsi86.c:218:12: error: 'ti_sn65dsi86_read_u16' defined but not used [-Werror=unused-function]
     218 | static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
         |            ^~~~~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors


vim +/ti_sn65dsi86_read_u16 +218 drivers/gpu/drm/bridge/ti-sn65dsi86.c

   217	
 > 218	static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
   219					 unsigned int reg, u16 *val)
   220	{
   221		u8 buf[2];
   222		int ret;
   223	
   224		ret = regmap_bulk_read(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
   225		if (ret)
   226			return ret;
   227	
   228		*val = buf[0] | (buf[1] << 8);
   229	
   230		return 0;
   231	}
   232	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32895 bytes --]

Re: [PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]

Hi Bjorn,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.15-rc6]
[also build test ERROR on next-20211027]
[cannot apply to thierry-reding-pwm/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
base:    519d81956ee277b4419c723adfb154603c2565ba
config: arm-buildonly-randconfig-r004-20211027 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/e5ac651e6c7c724f3cd33e0fe322e714c0796348
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
        git checkout e5ac651e6c7c724f3cd33e0fe322e714c0796348
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/ti-sn65dsi86.c:218:12: error: unused function 'ti_sn65dsi86_read_u16' [-Werror,-Wunused-function]
   static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
              ^
   1 error generated.


vim +/ti_sn65dsi86_read_u16 +218 drivers/gpu/drm/bridge/ti-sn65dsi86.c

   217	
 > 218	static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
   219					 unsigned int reg, u16 *val)
   220	{
   221		u8 buf[2];
   222		int ret;
   223	
   224		ret = regmap_bulk_read(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
   225		if (ret)
   226			return ret;
   227	
   228		*val = buf[0] | (buf[1] << 8);
   229	
   230		return 0;
   231	}
   232	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32667 bytes --]

Re: [PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2477 bytes --]

Hi Bjorn,

I love your patch! Yet something to improve:

[auto build test ERROR on v5.15-rc6]
[also build test ERROR on next-20211027]
[cannot apply to thierry-reding-pwm/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
base:    519d81956ee277b4419c723adfb154603c2565ba
config: arm-buildonly-randconfig-r004-20211027 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 5db7568a6a1fcb408eb8988abdaff2a225a8eb72)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/e5ac651e6c7c724f3cd33e0fe322e714c0796348
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
        git checkout e5ac651e6c7c724f3cd33e0fe322e714c0796348
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/bridge/ti-sn65dsi86.c:218:12: error: unused function 'ti_sn65dsi86_read_u16' [-Werror,-Wunused-function]
   static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
              ^
   1 error generated.


vim +/ti_sn65dsi86_read_u16 +218 drivers/gpu/drm/bridge/ti-sn65dsi86.c

   217	
 > 218	static int ti_sn65dsi86_read_u16(struct ti_sn65dsi86 *pdata,
   219					 unsigned int reg, u16 *val)
   220	{
   221		u8 buf[2];
   222		int ret;
   223	
   224		ret = regmap_bulk_read(pdata->regmap, reg, buf, ARRAY_SIZE(buf));
   225		if (ret)
   226			return ret;
   227	
   228		*val = buf[0] | (buf[1] << 8);
   229	
   230		return 0;
   231	}
   232	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 32667 bytes --]

Re: [PATCH v7 1/3] pwm: Introduce single-PWM of_xlate function

[Steev Klimaszewski]

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 <bjorn.andersson@linaro.org>
> Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Tested-by: Steev Klimaszewski <steev@kali.org>
> ---
>
> 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 <steev@kali.org>

Re: [PATCH v7 1/3] pwm: Introduce single-PWM of_xlate function

[Robert Foss]

On Tue, 26 Oct 2021 at 19:21, Steev Klimaszewski <steev@kali.org> 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 <bjorn.andersson@linaro.org>
> > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Tested-by: Steev Klimaszewski <steev@kali.org>
> > ---
> >
> > 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 <steev@kali.org>
>

Applied to drm-misc-next.

Re: [PATCH v7 1/3] pwm: Introduce single-PWM of_xlate function

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]

Hello,

On Wed, Oct 27, 2021 at 05:06:02PM +0200, Robert Foss wrote:
> On Tue, 26 Oct 2021 at 19:21, Steev Klimaszewski <steev@kali.org> 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 <bjorn.andersson@linaro.org>
> > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > Tested-by: Steev Klimaszewski <steev@kali.org>
> > > ---
> > >
> [...]
> 
> 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/20211025170925.3096444-1-bjorn.andersson@linaro.org

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 <steev@kali.org>".

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v7 1/3] pwm: Introduce single-PWM of_xlate function

[Robert Foss]

Hey Uwe

On Sat, 30 Oct 2021 at 11:27, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> 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 <steev@kali.org> 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 <bjorn.andersson@linaro.org>
> > > > Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > Tested-by: Steev Klimaszewski <steev@kali.org>
> > > > ---
> > > >
> > [...]
> >
> > 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/20211025170925.3096444-1-bjorn.andersson@linaro.org
>
> 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 <steev@kali.org>".

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=

Re: [PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211025170925.3096444-3-bjorn.andersson@linaro.org>
References: <20211025170925.3096444-3-bjorn.andersson@linaro.org>
TO: Bjorn Andersson <bjorn.andersson@linaro.org>

Hi Bjorn,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v5.15-rc6]
[also build test WARNING on next-20211027]
[cannot apply to thierry-reding-pwm/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
base:    519d81956ee277b4419c723adfb154603c2565ba
:::::: branch date: 2 days ago
:::::: commit date: 2 days ago
config: x86_64-randconfig-m001-20211027 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/ti-sn65dsi86.c:302 ti_sn_bridge_set_refclk_freq() error: buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5 (assuming for loop doesn't break)

vim +302 drivers/gpu/drm/bridge/ti-sn65dsi86.c

f7a5ee2cd3e23f Douglas Anderson 2021-04-23  271  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  272  static void ti_sn_bridge_set_refclk_freq(struct ti_sn65dsi86 *pdata)
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  273  {
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  274  	int i;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  275  	u32 refclk_rate;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  276  	const u32 *refclk_lut;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  277  	size_t refclk_lut_size;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  278  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  279  	if (pdata->refclk) {
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  280  		refclk_rate = clk_get_rate(pdata->refclk);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  281  		refclk_lut = ti_sn_bridge_refclk_lut;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  282  		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  283  		clk_prepare_enable(pdata->refclk);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  284  	} else {
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  285  		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  286  		refclk_lut = ti_sn_bridge_dsiclk_lut;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  287  		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  288  	}
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  289  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  290  	/* for i equals to refclk_lut_size means default frequency */
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  291  	for (i = 0; i < refclk_lut_size; i++)
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  292  		if (refclk_lut[i] == refclk_rate)
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  293  			break;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  294  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  295  	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  296  			   REFCLK_FREQ(i));
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  297  
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  298  	/*
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  299  	 * The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  300  	 * regardless of its actual sourcing.
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  301  	 */
e5ac651e6c7c72 Bjorn Andersson  2021-10-25 @302  	pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  303  }
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  304  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37412 bytes --]

Re: [PATCH v7 3/3] drm/bridge: ti-sn65dsi86: Implement the pwm_chip

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 3582 bytes --]

Hi Bjorn,

url:    https://github.com/0day-ci/linux/commits/Bjorn-Andersson/pwm-Introduce-single-PWM-of_xlate-function/20211026-011243
base:    519d81956ee277b4419c723adfb154603c2565ba
config: x86_64-randconfig-m001-20211027 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

smatch warnings:
drivers/gpu/drm/bridge/ti-sn65dsi86.c:302 ti_sn_bridge_set_refclk_freq() error: buffer overflow 'ti_sn_bridge_refclk_lut' 5 <= 5 (assuming for loop doesn't break)

vim +302 drivers/gpu/drm/bridge/ti-sn65dsi86.c

f7a5ee2cd3e23f Douglas Anderson 2021-04-23  272  static void ti_sn_bridge_set_refclk_freq(struct ti_sn65dsi86 *pdata)
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  273  {
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  274  	int i;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  275  	u32 refclk_rate;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  276  	const u32 *refclk_lut;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  277  	size_t refclk_lut_size;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  278  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  279  	if (pdata->refclk) {
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  280  		refclk_rate = clk_get_rate(pdata->refclk);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  281  		refclk_lut = ti_sn_bridge_refclk_lut;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  282  		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_refclk_lut);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  283  		clk_prepare_enable(pdata->refclk);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  284  	} else {
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  285  		refclk_rate = ti_sn_bridge_get_dsi_freq(pdata) * 1000;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  286  		refclk_lut = ti_sn_bridge_dsiclk_lut;
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  287  		refclk_lut_size = ARRAY_SIZE(ti_sn_bridge_dsiclk_lut);
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  288  	}
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  289  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  290  	/* for i equals to refclk_lut_size means default frequency */
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  291  	for (i = 0; i < refclk_lut_size; i++)
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  292  		if (refclk_lut[i] == refclk_rate)
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  293  			break;

The warning is complaining the if (i == refclk_lut_size) after the end
of a loop then it leads to a read overflow.

f7a5ee2cd3e23f Douglas Anderson 2021-04-23  294  
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  295  	regmap_update_bits(pdata->regmap, SN_DPPLL_SRC_REG, REFCLK_FREQ_MASK,
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  296  			   REFCLK_FREQ(i));
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  297  
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  298  	/*
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  299  	 * The PWM refclk is based on the value written to SN_DPPLL_SRC_REG,
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  300  	 * regardless of its actual sourcing.
e5ac651e6c7c72 Bjorn Andersson  2021-10-25  301  	 */
e5ac651e6c7c72 Bjorn Andersson  2021-10-25 @302  	pdata->pwm_refclk_freq = ti_sn_bridge_refclk_lut[i];
                                                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
f7a5ee2cd3e23f Douglas Anderson 2021-04-23  303  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org