[PATCH 1/4] drm/bridge: ti-sn65dsi86: Don't compile GPIO bits if not CONFIG_OF_GPIO

[PATCH 1/4] drm/bridge: ti-sn65dsi86: Don't compile GPIO bits if not CONFIG_OF_GPIO

[Douglas Anderson]

The kernel test robot noted that if "OF" is defined (which is needed
to select DRM_TI_SN65DSI86 at all) but not OF_GPIO that we'd get
compile failures because some of the members that we access in "struct
gpio_chip" are only defined "#if defined(CONFIG_OF_GPIO)".

All the GPIO bits in the driver are all nicely separated out.  We'll
guard them with the same "#if defined" that the header has and add a
little stub function if OF_GPIO is not defined.

Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 2240e9973178..6fa7e10b31af 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -151,8 +151,10 @@ struct ti_sn_bridge {
 	u8				ln_assign;
 	u8				ln_polrs;
 
+#if defined(CONFIG_OF_GPIO)
 	struct gpio_chip		gchip;
 	DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
+#endif
 };
 
 static const struct regmap_range ti_sn_bridge_volatile_ranges[] = {
@@ -925,6 +927,8 @@ static int ti_sn_bridge_parse_dsi_host(struct ti_sn_bridge *pdata)
 	return 0;
 }
 
+#if defined(CONFIG_OF_GPIO)
+
 static int tn_sn_bridge_of_xlate(struct gpio_chip *chip,
 				 const struct of_phandle_args *gpiospec,
 				 u32 *flags)
@@ -1092,6 +1096,15 @@ static int ti_sn_setup_gpio_controller(struct ti_sn_bridge *pdata)
 	return ret;
 }
 
+#else
+
+static inline int ti_sn_setup_gpio_controller(struct ti_sn_bridge *pdata)
+{
+	return 0;
+}
+
+#endif
+
 static void ti_sn_bridge_parse_lanes(struct ti_sn_bridge *pdata,
 				     struct device_node *np)
 {
-- 
2.27.0.278.ge193c7cf3a9-goog

[PATCH 2/4] drm/bridge: ti-sn65dsi86: Don't use kernel-doc comment for local array

[Douglas Anderson]

When building we were getting an error:

  warning: cannot understand function prototype:
    'const unsigned int ti_sn_bridge_dp_rate_lut[] = '

Arrays aren't supposed to be marked with "/**" kerneldoc comments.  Fix.

Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 6fa7e10b31af..fca7c2a0bcf9 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -504,7 +504,7 @@ static unsigned int ti_sn_bridge_get_bpp(struct ti_sn_bridge *pdata)
 		return 24;
 }
 
-/**
+/*
  * LUT index corresponds to register value and
  * LUT values corresponds to dp data rate supported
  * by the bridge in Mbps unit.
-- 
2.27.0.278.ge193c7cf3a9-goog

[PATCH 3/4] drm/bridge: ti-sn65dsi86: Fix kernel-doc typo ln_polr => ln_polrs

[Douglas Anderson]

This fixes a kernel doc warning due to a typo:
  warning: Function parameter or member 'ln_polrs' not described in 'ti_sn_bridge'

Fixes: 5bebaeadb30e ("drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index fca7c2a0bcf9..1080e4f9df96 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -122,7 +122,7 @@
  * @supplies:     Data for bulk enabling/disabling our regulators.
  * @dp_lanes:     Count of dp_lanes we're using.
  * @ln_assign:    Value to program to the LN_ASSIGN register.
- * @ln_polr:      Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
+ * @ln_polrs:     Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
  *
  * @gchip:        If we expose our GPIOs, this is used.
  * @gchip_output: A cache of whether we've set GPIOs to output.  This
-- 
2.27.0.278.ge193c7cf3a9-goog

[PATCH 4/4] drm/bridge: ti-sn65dsi86: Check the regmap return value when setting a GPIO

[Douglas Anderson]

The ti_sn_bridge_gpio_set() got the return value of
regmap_update_bits() but didn't check it.  The function can't return
an error value, but we should at least print a warning if it didn't
work.

This fixes a compiler warning about setting "ret" but not using it.

Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1080e4f9df96..526add27dc03 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -999,6 +999,9 @@ static void ti_sn_bridge_gpio_set(struct gpio_chip *chip, unsigned int offset,
 	ret = regmap_update_bits(pdata->regmap, SN_GPIO_IO_REG,
 				 BIT(SN_GPIO_OUTPUT_SHIFT + offset),
 				 val << (SN_GPIO_OUTPUT_SHIFT + offset));
+	if (ret)
+		dev_warn(pdata->dev,
+			 "Failed to set bridge GPIO %d: %d\n", offset, ret);
 }
 
 static int ti_sn_bridge_gpio_direction_input(struct gpio_chip *chip,
-- 
2.27.0.278.ge193c7cf3a9-goog

Re: [PATCH 1/4] drm/bridge: ti-sn65dsi86: Don't compile GPIO bits if not CONFIG_OF_GPIO

[Stephen Boyd]

Quoting Douglas Anderson (2020-06-08 10:48:32)
> The kernel test robot noted that if "OF" is defined (which is needed
> to select DRM_TI_SN65DSI86 at all) but not OF_GPIO that we'd get
> compile failures because some of the members that we access in "struct
> gpio_chip" are only defined "#if defined(CONFIG_OF_GPIO)".
> 
> All the GPIO bits in the driver are all nicely separated out.  We'll
> guard them with the same "#if defined" that the header has and add a
> little stub function if OF_GPIO is not defined.
> 
> Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH 2/4] drm/bridge: ti-sn65dsi86: Don't use kernel-doc comment for local array

[Stephen Boyd]

Quoting Douglas Anderson (2020-06-08 10:48:33)
> When building we were getting an error:
> 
>   warning: cannot understand function prototype:
>     'const unsigned int ti_sn_bridge_dp_rate_lut[] = '
> 
> Arrays aren't supposed to be marked with "/**" kerneldoc comments.  Fix.
> 
> Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH 4/4] drm/bridge: ti-sn65dsi86: Check the regmap return value when setting a GPIO

[Stephen Boyd]

Quoting Douglas Anderson (2020-06-08 10:48:35)
> The ti_sn_bridge_gpio_set() got the return value of
> regmap_update_bits() but didn't check it.  The function can't return
> an error value, but we should at least print a warning if it didn't
> work.
> 
> This fixes a compiler warning about setting "ret" but not using it.
> 
> Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 1080e4f9df96..526add27dc03 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -999,6 +999,9 @@ static void ti_sn_bridge_gpio_set(struct gpio_chip *chip, unsigned int offset,
>         ret = regmap_update_bits(pdata->regmap, SN_GPIO_IO_REG,
>                                  BIT(SN_GPIO_OUTPUT_SHIFT + offset),
>                                  val << (SN_GPIO_OUTPUT_SHIFT + offset));
> +       if (ret)
> +               dev_warn(pdata->dev,
> +                        "Failed to set bridge GPIO %d: %d\n", offset, ret);

GPIO %u because it's unsigned?

Re: [PATCH 3/4] drm/bridge: ti-sn65dsi86: Fix kernel-doc typo ln_polr => ln_polrs

[Stephen Boyd]

Quoting Douglas Anderson (2020-06-08 10:48:34)
> This fixes a kernel doc warning due to a typo:
>   warning: Function parameter or member 'ln_polrs' not described in 'ti_sn_bridge'
> 
> Fixes: 5bebaeadb30e ("drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH 4/4] drm/bridge: ti-sn65dsi86: Check the regmap return value when setting a GPIO

[Doug Anderson]

Hi,

On Thu, Jun 11, 2020 at 2:58 AM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Douglas Anderson (2020-06-08 10:48:35)
> > The ti_sn_bridge_gpio_set() got the return value of
> > regmap_update_bits() but didn't check it.  The function can't return
> > an error value, but we should at least print a warning if it didn't
> > work.
> >
> > This fixes a compiler warning about setting "ret" but not using it.
> >
> > Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > index 1080e4f9df96..526add27dc03 100644
> > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> > @@ -999,6 +999,9 @@ static void ti_sn_bridge_gpio_set(struct gpio_chip *chip, unsigned int offset,
> >         ret = regmap_update_bits(pdata->regmap, SN_GPIO_IO_REG,
> >                                  BIT(SN_GPIO_OUTPUT_SHIFT + offset),
> >                                  val << (SN_GPIO_OUTPUT_SHIFT + offset));
> > +       if (ret)
> > +               dev_warn(pdata->dev,
> > +                        "Failed to set bridge GPIO %d: %d\n", offset, ret);
>
> GPIO %u because it's unsigned?

Sure.  I'll plan to spin tomorrow in case anyone else has any
feedback.  If any maintainer would prefer me not to spin and would
rather fix this when applying, please shout and I won't send out a v2.

-Doug

Re: [PATCH 1/4] drm/bridge: ti-sn65dsi86: Don't compile GPIO bits if not CONFIG_OF_GPIO

[Linus Walleij]

On Mon, Jun 8, 2020 at 7:48 PM Douglas Anderson <dianders@chromium.org> wrote:

> The kernel test robot noted that if "OF" is defined (which is needed
> to select DRM_TI_SN65DSI86 at all) but not OF_GPIO that we'd get
> compile failures because some of the members that we access in "struct
> gpio_chip" are only defined "#if defined(CONFIG_OF_GPIO)".
>
> All the GPIO bits in the driver are all nicely separated out.  We'll
> guard them with the same "#if defined" that the header has and add a
> little stub function if OF_GPIO is not defined.
>
> Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>

Fair enough,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 4/4] drm/bridge: ti-sn65dsi86: Check the regmap return value when setting a GPIO

[Neil Armstrong]

On 11/06/2020 16:34, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jun 11, 2020 at 2:58 AM Stephen Boyd <swboyd@chromium.org> wrote:
>>
>> Quoting Douglas Anderson (2020-06-08 10:48:35)
>>> The ti_sn_bridge_gpio_set() got the return value of
>>> regmap_update_bits() but didn't check it.  The function can't return
>>> an error value, but we should at least print a warning if it didn't
>>> work.
>>>
>>> This fixes a compiler warning about setting "ret" but not using it.
>>>
>>> Fixes: 27ed2b3f22ed ("drm/bridge: ti-sn65dsi86: Export bridge GPIOs to Linux")
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>> ---
>>>
>>>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> index 1080e4f9df96..526add27dc03 100644
>>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>> @@ -999,6 +999,9 @@ static void ti_sn_bridge_gpio_set(struct gpio_chip *chip, unsigned int offset,
>>>         ret = regmap_update_bits(pdata->regmap, SN_GPIO_IO_REG,
>>>                                  BIT(SN_GPIO_OUTPUT_SHIFT + offset),
>>>                                  val << (SN_GPIO_OUTPUT_SHIFT + offset));
>>> +       if (ret)
>>> +               dev_warn(pdata->dev,
>>> +                        "Failed to set bridge GPIO %d: %d\n", offset, ret);
>>
>> GPIO %u because it's unsigned?
> 
> Sure.  I'll plan to spin tomorrow in case anyone else has any
> feedback.  If any maintainer would prefer me not to spin and would
> rather fix this when applying, please shout and I won't send out a v2.
> 
> -Doug
> 

Yes please respin, ping me on IRC if I forget to apply..

Neil