The i2c bindings in the kernel tree describe support for 10 bit
addressing, which must be indicated with the I2C_TEN_BIT_ADDRESS flag.
When this is set the address can be up to 10 bits. When it is not set
the address is a maximum of 7 bits.
See Documentation/devicetree/bindings/i2c/i2c.txt.
Take into account this flag when checking the address is valid.
Signed-off-by: Joel Stanley <[email protected]>
---
v2: Mask reg when checking the 10-bit size
---
checks.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/checks.c b/checks.c
index feb1721f2603..3fe979a63290 100644
--- a/checks.c
+++ b/checks.c
@@ -1023,6 +1023,7 @@ static void check_i2c_bus_bridge(struct check *c, struct dt_info *dti, struct no
WARNING(i2c_bus_bridge, check_i2c_bus_bridge, NULL, &addr_size_cells);
#define I2C_OWN_SLAVE_ADDRESS (1 << 30)
+#define I2C_TEN_BIT_ADDRESS (1 << 31)
static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node *node)
{
@@ -1057,10 +1058,13 @@ static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node
reg = fdt32_to_cpu(*(cells++));
/* Ignore I2C_OWN_SLAVE_ADDRESS */
reg &= ~I2C_OWN_SLAVE_ADDRESS;
- if (reg > 0x3ff)
+
+ if ((reg & I2C_TEN_BIT_ADDRESS) && ((reg & ~I2C_TEN_BIT_ADDRESS) > 0x3ff))
FAIL_PROP(c, dti, node, prop, "I2C address must be less than 10-bits, got \"0x%x\"",
reg);
-
+ else if (reg > 0x7f)
+ FAIL_PROP(c, dti, node, prop, "I2C address must be less than 7-bits, got \"0x%x\". Set I2C_TEN_BIT_ADDRESS for 10 bit addresses or fix the property",
+ reg);
}
}
WARNING(i2c_bus_reg, check_i2c_bus_reg, NULL, ®_format, &i2c_bus_bridge);
--
2.26.2
On Thu, May 28, 2020 at 06:26:50PM +0930, Joel Stanley wrote:
> The i2c bindings in the kernel tree describe support for 10 bit
> addressing, which must be indicated with the I2C_TEN_BIT_ADDRESS flag.
> When this is set the address can be up to 10 bits. When it is not set
> the address is a maximum of 7 bits.
>
> See Documentation/devicetree/bindings/i2c/i2c.txt.
>
> Take into account this flag when checking the address is valid.
>
> Signed-off-by: Joel Stanley <[email protected]>
> ---
> v2: Mask reg when checking the 10-bit size
> ---
> checks.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/checks.c b/checks.c
> index feb1721f2603..3fe979a63290 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1023,6 +1023,7 @@ static void check_i2c_bus_bridge(struct check *c, struct dt_info *dti, struct no
> WARNING(i2c_bus_bridge, check_i2c_bus_bridge, NULL, &addr_size_cells);
>
> #define I2C_OWN_SLAVE_ADDRESS (1 << 30)
> +#define I2C_TEN_BIT_ADDRESS (1 << 31)
As Andy neatly pointed out here:
https://lore.kernel.org/lkml/[email protected]/
(1 << 31) is UB.
-Sergey
>
> static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node *node)
> {
> @@ -1057,10 +1058,13 @@ static void check_i2c_bus_reg(struct check *c, struct dt_info *dti, struct node
> reg = fdt32_to_cpu(*(cells++));
> /* Ignore I2C_OWN_SLAVE_ADDRESS */
> reg &= ~I2C_OWN_SLAVE_ADDRESS;
> - if (reg > 0x3ff)
> +
> + if ((reg & I2C_TEN_BIT_ADDRESS) && ((reg & ~I2C_TEN_BIT_ADDRESS) > 0x3ff))
> FAIL_PROP(c, dti, node, prop, "I2C address must be less than 10-bits, got \"0x%x\"",
> reg);
> -
> + else if (reg > 0x7f)
> + FAIL_PROP(c, dti, node, prop, "I2C address must be less than 7-bits, got \"0x%x\". Set I2C_TEN_BIT_ADDRESS for 10 bit addresses or fix the property",
> + reg);
> }
> }
> WARNING(i2c_bus_reg, check_i2c_bus_reg, NULL, ®_format, &i2c_bus_bridge);
> --
> 2.26.2
>
On Tue, Jun 2, 2020 at 11:03 AM Serge Semin
<[email protected]> wrote:
> On Thu, May 28, 2020 at 06:26:50PM +0930, Joel Stanley wrote:
...
> > +#define I2C_TEN_BIT_ADDRESS (1 << 31)
>
> As Andy neatly pointed out here:
> https://lore.kernel.org/lkml/[email protected]/
> (1 << 31) is UB.
Thanks, Serge. Yes, we have to use 1U in the definitions (for 31 is
necessary, for the rest is for the sake of consistency).
--
With Best Regards,
Andy Shevchenko
On Tue, Jun 02, 2020 at 11:28:05AM +0300, Andy Shevchenko wrote:
> On Tue, Jun 2, 2020 at 11:03 AM Serge Semin
> <[email protected]> wrote:
> > On Thu, May 28, 2020 at 06:26:50PM +0930, Joel Stanley wrote:
>
> ...
>
> > > +#define I2C_TEN_BIT_ADDRESS (1 << 31)
> >
> > As Andy neatly pointed out here:
> > https://lore.kernel.org/lkml/[email protected]/
> > (1 << 31) is UB.
>
> Thanks, Serge. Yes, we have to use 1U in the definitions (for 31 is
> necessary, for the rest is for the sake of consistency).
Joel, I know it seems trivial, but I'm a bit flat out right now. Can
you please resend with the 1U fix applied.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson