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 <[email protected]>
Signed-off-by: Douglas Anderson <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
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 <[email protected]>
> ---
>
> 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?
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 <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
Hi,
On Thu, Jun 11, 2020 at 2:58 AM Stephen Boyd <[email protected]> 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 <[email protected]>
> > ---
> >
> > 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
On Mon, Jun 8, 2020 at 7:48 PM Douglas Anderson <[email protected]> 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 <[email protected]>
> Signed-off-by: Douglas Anderson <[email protected]>
Fair enough,
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
On 11/06/2020 16:34, Doug Anderson wrote:
> Hi,
>
> On Thu, Jun 11, 2020 at 2:58 AM Stephen Boyd <[email protected]> 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 <[email protected]>
>>> ---
>>>
>>> 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