linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phy-rockchip-inno-hdmi: Fix RK3328_TERM_RESISTOR_CALIB_SPEED_7_0's third value
@ 2019-08-07 19:23 Nathan Chancellor
  2019-08-07 21:02 ` Heiko Stübner
  2019-08-20 18:12 ` Nathan Chancellor
  0 siblings, 2 replies; 3+ messages in thread
From: Nathan Chancellor @ 2019-08-07 19:23 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Heiko Stuebner
  Cc: linux-kernel, linux-arm-kernel, linux-rockchip,
	Nathan Chancellor, Andrzej Hajda, Guenter Roeck,
	kernelci . org bot, Naresh Kamboju, Robin Murphy

After commit "linux/bits.h: Add compile time sanity check of GENMASK
inputs" [1], arm64 defconfig builds started failing:

In file included from ../include/linux/bits.h:22,
                 from ../include/linux/bitops.h:5,
                 from ../include/linux/kernel.h:12,
                 from ../include/linux/clk.h:13,
                 from ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:9:
../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c: In function 'inno_hdmi_phy_rk3328_power_on':
../include/linux/build_bug.h:16:45: error: negative width in bit-field '<anonymous>'
   16 | #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
      |                                             ^
../include/linux/bits.h:24:18: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
   24 |  ((unsigned long)BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
      |                  ^~~~~~~~~~~~~~~~~
../include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
   39 |  (GENMASK_INPUT_CHECK(high, low) + __GENMASK(high, low))
      |   ^~~~~~~~~~~~~~~~~~~
../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:24:42: note: in expansion of macro 'GENMASK'
   24 | #define UPDATE(x, h, l)  (((x) << (l)) & GENMASK((h), (l)))
      |                                          ^~~~~~~
../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:201:50: note: in expansion of macro 'UPDATE'
  201 | #define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)  UPDATE(x, 7, 9)
      |                                                  ^~~~~~
../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:1046:26: note: in expansion of macro 'RK3328_TERM_RESISTOR_CALIB_SPEED_7_0'
 1046 |   inno_write(inno, 0xc6, RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(v));
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

As pointed out by Robin and Guenter, inno_write's val argument is an
8-bit value so having a mask larger than that doesn't make sense. This
also matches the rest of the *_7_0 macros in this driver.

[1]: https://lore.kernel.org/lkml/20190801230358.4193-2-rikard.falkeborn@gmail.com/

Reported-by: Andrzej Hajda <a.hajda@samsung.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Reported-by: kernelci.org bot <bot@kernelci.org>
Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Suggested-by: Robin Murphy <robin.murphy@arm.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
index b10a84cab4a7..2b97fb1185a0 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
@@ -198,7 +198,7 @@
 #define RK3328_BYPASS_TERM_RESISTOR_CALIB		BIT(7)
 #define RK3328_TERM_RESISTOR_CALIB_SPEED_14_8(x)	UPDATE((x) >> 8, 6, 0)
 /* REG:0xc6 */
-#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 9)
+#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 0)
 /* REG:0xc7 */
 #define RK3328_TERM_RESISTOR_50				UPDATE(0, 2, 1)
 #define RK3328_TERM_RESISTOR_62_5			UPDATE(1, 2, 1)
-- 
2.23.0.rc1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy-rockchip-inno-hdmi: Fix RK3328_TERM_RESISTOR_CALIB_SPEED_7_0's third value
  2019-08-07 19:23 [PATCH] phy-rockchip-inno-hdmi: Fix RK3328_TERM_RESISTOR_CALIB_SPEED_7_0's third value Nathan Chancellor
@ 2019-08-07 21:02 ` Heiko Stübner
  2019-08-20 18:12 ` Nathan Chancellor
  1 sibling, 0 replies; 3+ messages in thread
From: Heiko Stübner @ 2019-08-07 21:02 UTC (permalink / raw)
  To: Nathan Chancellor, Kishon Vijay Abraham I
  Cc: linux-kernel, linux-arm-kernel, linux-rockchip, Andrzej Hajda,
	Guenter Roeck, kernelci . org bot, Naresh Kamboju, Robin Murphy

Am Mittwoch, 7. August 2019, 21:23:05 CEST schrieb Nathan Chancellor:
> After commit "linux/bits.h: Add compile time sanity check of GENMASK
> inputs" [1], arm64 defconfig builds started failing:
> 
> In file included from ../include/linux/bits.h:22,
>                  from ../include/linux/bitops.h:5,
>                  from ../include/linux/kernel.h:12,
>                  from ../include/linux/clk.h:13,
>                  from ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:9:
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c: In function 'inno_hdmi_phy_rk3328_power_on':
> ../include/linux/build_bug.h:16:45: error: negative width in bit-field '<anonymous>'
>    16 | #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
>       |                                             ^
> ../include/linux/bits.h:24:18: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
>    24 |  ((unsigned long)BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
>       |                  ^~~~~~~~~~~~~~~~~
> ../include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
>    39 |  (GENMASK_INPUT_CHECK(high, low) + __GENMASK(high, low))
>       |   ^~~~~~~~~~~~~~~~~~~
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:24:42: note: in expansion of macro 'GENMASK'
>    24 | #define UPDATE(x, h, l)  (((x) << (l)) & GENMASK((h), (l)))
>       |                                          ^~~~~~~
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:201:50: note: in expansion of macro 'UPDATE'
>   201 | #define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)  UPDATE(x, 7, 9)
>       |                                                  ^~~~~~
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:1046:26: note: in expansion of macro 'RK3328_TERM_RESISTOR_CALIB_SPEED_7_0'
>  1046 |   inno_write(inno, 0xc6, RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(v));
>       |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> As pointed out by Robin and Guenter, inno_write's val argument is an
> 8-bit value so having a mask larger than that doesn't make sense. This
> also matches the rest of the *_7_0 macros in this driver.
> 
> [1]: https://lore.kernel.org/lkml/20190801230358.4193-2-rikard.falkeborn@gmail.com/
> 
> Reported-by: Andrzej Hajda <a.hajda@samsung.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Reported-by: kernelci.org bot <bot@kernelci.org>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

on a rk3328-rock64 hdmi output still works
Tested-by: Heiko Stuebner <heiko@sntech.de>

@Kishon: Would probably be good to get this fast into 5.3-rc.


Heiko


>  drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> index b10a84cab4a7..2b97fb1185a0 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> @@ -198,7 +198,7 @@
>  #define RK3328_BYPASS_TERM_RESISTOR_CALIB		BIT(7)
>  #define RK3328_TERM_RESISTOR_CALIB_SPEED_14_8(x)	UPDATE((x) >> 8, 6, 0)
>  /* REG:0xc6 */
> -#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 9)
> +#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 0)
>  /* REG:0xc7 */
>  #define RK3328_TERM_RESISTOR_50				UPDATE(0, 2, 1)
>  #define RK3328_TERM_RESISTOR_62_5			UPDATE(1, 2, 1)
> 





^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] phy-rockchip-inno-hdmi: Fix RK3328_TERM_RESISTOR_CALIB_SPEED_7_0's third value
  2019-08-07 19:23 [PATCH] phy-rockchip-inno-hdmi: Fix RK3328_TERM_RESISTOR_CALIB_SPEED_7_0's third value Nathan Chancellor
  2019-08-07 21:02 ` Heiko Stübner
@ 2019-08-20 18:12 ` Nathan Chancellor
  1 sibling, 0 replies; 3+ messages in thread
From: Nathan Chancellor @ 2019-08-20 18:12 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Heiko Stuebner
  Cc: linux-kernel, linux-arm-kernel, linux-rockchip, Andrzej Hajda,
	Guenter Roeck, kernelci . org bot, Naresh Kamboju, Robin Murphy

On Wed, Aug 07, 2019 at 12:23:05PM -0700, Nathan Chancellor wrote:
> After commit "linux/bits.h: Add compile time sanity check of GENMASK
> inputs" [1], arm64 defconfig builds started failing:
> 
> In file included from ../include/linux/bits.h:22,
>                  from ../include/linux/bitops.h:5,
>                  from ../include/linux/kernel.h:12,
>                  from ../include/linux/clk.h:13,
>                  from ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:9:
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c: In function 'inno_hdmi_phy_rk3328_power_on':
> ../include/linux/build_bug.h:16:45: error: negative width in bit-field '<anonymous>'
>    16 | #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
>       |                                             ^
> ../include/linux/bits.h:24:18: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
>    24 |  ((unsigned long)BUILD_BUG_ON_ZERO(__builtin_choose_expr( \
>       |                  ^~~~~~~~~~~~~~~~~
> ../include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
>    39 |  (GENMASK_INPUT_CHECK(high, low) + __GENMASK(high, low))
>       |   ^~~~~~~~~~~~~~~~~~~
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:24:42: note: in expansion of macro 'GENMASK'
>    24 | #define UPDATE(x, h, l)  (((x) << (l)) & GENMASK((h), (l)))
>       |                                          ^~~~~~~
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:201:50: note: in expansion of macro 'UPDATE'
>   201 | #define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)  UPDATE(x, 7, 9)
>       |                                                  ^~~~~~
> ../drivers/phy/rockchip/phy-rockchip-inno-hdmi.c:1046:26: note: in expansion of macro 'RK3328_TERM_RESISTOR_CALIB_SPEED_7_0'
>  1046 |   inno_write(inno, 0xc6, RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(v));
>       |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> As pointed out by Robin and Guenter, inno_write's val argument is an
> 8-bit value so having a mask larger than that doesn't make sense. This
> also matches the rest of the *_7_0 macros in this driver.
> 
> [1]: https://lore.kernel.org/lkml/20190801230358.4193-2-rikard.falkeborn@gmail.com/
> 
> Reported-by: Andrzej Hajda <a.hajda@samsung.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Reported-by: kernelci.org bot <bot@kernelci.org>
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Suggested-by: Robin Murphy <robin.murphy@arm.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> index b10a84cab4a7..2b97fb1185a0 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c
> @@ -198,7 +198,7 @@
>  #define RK3328_BYPASS_TERM_RESISTOR_CALIB		BIT(7)
>  #define RK3328_TERM_RESISTOR_CALIB_SPEED_14_8(x)	UPDATE((x) >> 8, 6, 0)
>  /* REG:0xc6 */
> -#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 9)
> +#define RK3328_TERM_RESISTOR_CALIB_SPEED_7_0(x)		UPDATE(x, 7, 0)
>  /* REG:0xc7 */
>  #define RK3328_TERM_RESISTOR_50				UPDATE(0, 2, 1)
>  #define RK3328_TERM_RESISTOR_62_5			UPDATE(1, 2, 1)
> -- 
> 2.23.0.rc1
> 

There was a v3 of linux/bits.h: Add compile time sanity check of GENMASK
inputs sent and this needs to be picked up to avoid build breakage when
that is applied. Could someone please pick this up?

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-08-20 18:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 19:23 [PATCH] phy-rockchip-inno-hdmi: Fix RK3328_TERM_RESISTOR_CALIB_SPEED_7_0's third value Nathan Chancellor
2019-08-07 21:02 ` Heiko Stübner
2019-08-20 18:12 ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).