[PATCH linux dev-5.4 0/2] i2c: fsi: Fixes for systems with more ports

[PATCH linux dev-5.4 0/2] i2c: fsi: Fixes for systems with more ports

[Eddie James]

Fix two things that cause issues on systems with lots of ports off the FSI
I2C master.

Eddie James (2):
  i2c: fsi: Fix the port number field in status register
  i2c: fsi: Prevent adding adapters for ports without dts nodes

 drivers/i2c/busses/i2c-fsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.24.0

[PATCH linux dev-5.4 1/2] i2c: fsi: Fix the port number field in status register

[Eddie James]

The port number field in the status register was not correct, so fix it.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/i2c/busses/i2c-fsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index e0c256922d4f..977d6f524649 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -98,7 +98,7 @@
 #define I2C_STAT_DAT_REQ	BIT(25)
 #define I2C_STAT_CMD_COMP	BIT(24)
 #define I2C_STAT_STOP_ERR	BIT(23)
-#define I2C_STAT_MAX_PORT	GENMASK(19, 16)
+#define I2C_STAT_MAX_PORT	GENMASK(22, 16)
 #define I2C_STAT_ANY_INT	BIT(15)
 #define I2C_STAT_SCL_IN		BIT(11)
 #define I2C_STAT_SDA_IN		BIT(10)
-- 
2.24.0

Re: [PATCH linux dev-5.4 1/2] i2c: fsi: Fix the port number field in status register

[Joel Stanley]

On Tue, 19 May 2020 at 19:39, Eddie James <eajames@linux.ibm.com> wrote:
>
> The port number field in the status register was not correct, so fix it.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

Merged into dev-5.4.

Can you please send this upstream with a fixes tag?

Cheers,

Joel

> ---
>  drivers/i2c/busses/i2c-fsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index e0c256922d4f..977d6f524649 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -98,7 +98,7 @@
>  #define I2C_STAT_DAT_REQ       BIT(25)
>  #define I2C_STAT_CMD_COMP      BIT(24)
>  #define I2C_STAT_STOP_ERR      BIT(23)
> -#define I2C_STAT_MAX_PORT      GENMASK(19, 16)
> +#define I2C_STAT_MAX_PORT      GENMASK(22, 16)
>  #define I2C_STAT_ANY_INT       BIT(15)
>  #define I2C_STAT_SCL_IN                BIT(11)
>  #define I2C_STAT_SDA_IN                BIT(10)
> --
> 2.24.0
>

[PATCH linux dev-5.4 2/2] i2c: fsi: Prevent adding adapters for ports without dts nodes

[Eddie James]

Ports should be defined in the devicetree if they are to be enabled on
the system.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/i2c/busses/i2c-fsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
index 977d6f524649..d6ea502521a5 100644
--- a/drivers/i2c/busses/i2c-fsi.c
+++ b/drivers/i2c/busses/i2c-fsi.c
@@ -703,7 +703,7 @@ static int fsi_i2c_probe(struct device *dev)
 
 	for (port_no = 0; port_no < ports; port_no++) {
 		np = fsi_i2c_find_port_of_node(dev->of_node, port_no);
-		if (np && !of_device_is_available(np))
+		if (!np || !of_device_is_available(np))
 			continue;
 
 		port = kzalloc(sizeof(*port), GFP_KERNEL);
-- 
2.24.0

Re: [PATCH linux dev-5.4 2/2] i2c: fsi: Prevent adding adapters for ports without dts nodes

[Joel Stanley]

On Tue, 19 May 2020 at 19:39, Eddie James <eajames@linux.ibm.com> wrote:
>
> Ports should be defined in the devicetree if they are to be enabled on
> the system.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/i2c/busses/i2c-fsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-fsi.c b/drivers/i2c/busses/i2c-fsi.c
> index 977d6f524649..d6ea502521a5 100644
> --- a/drivers/i2c/busses/i2c-fsi.c
> +++ b/drivers/i2c/busses/i2c-fsi.c
> @@ -703,7 +703,7 @@ static int fsi_i2c_probe(struct device *dev)
>
>         for (port_no = 0; port_no < ports; port_no++) {
>                 np = fsi_i2c_find_port_of_node(dev->of_node, port_no);
> -               if (np && !of_device_is_available(np))
> +               if (!np || !of_device_is_available(np))

I thought this was wrong, but the important part is that
of_device_is_available() returns true if it can't find the status
property (ie np is null), so we need to check for np being NULL, and
if that's false we can check if it's "available".

We could write it like this if you agree it's clearer:

if (!np)
   /* Port not defined in device tree */
   continue;
if (of_device_is_available())
   continue;

Or I can take your patch as-is. Let me know.

Cheers,

Joel




>                         continue;
>
>                 port = kzalloc(sizeof(*port), GFP_KERNEL);
> --
> 2.24.0
>