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) {
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.
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
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
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
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,
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?
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
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
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
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
dri-devel@lists.freedesktop.org
-
Doug Anderson -
Douglas Anderson
-
Linus Walleij -
Neil Armstrong -
Stephen Boyd