[PATCH] gpio: turris_omnia_mcu: Fix registering gpios

Currently all GPIOs supported by CMD_EXT_CONTROL/CMD_GET_EXT_CONTROL_STATUS commands (last 16 GPIOs) are available only when FEAT_PERIPH_MCU feature bit is set. So do not register these GPIOs by U-Boot driver when this feature bit is not set, so U-Boot 'gpio' command would see only GPIOs which really exists.
Fixes: 5e4d24ccc115 ("gpio: Add Turris Omnia MCU driver") Signed-off-by: Pali Rohár pali@kernel.org --- drivers/gpio/turris_omnia_mcu.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c index 986ccde6bc70..2d2bf2d1dd69 100644 --- a/drivers/gpio/turris_omnia_mcu.c +++ b/drivers/gpio/turris_omnia_mcu.c @@ -16,6 +16,8 @@ enum commands_e {
/* available if FEAT_EXT_CMDS bit is set in features */ CMD_GET_EXT_STATUS_DWORD = 0x11, + + /* available if FEAT_EXT_CMDS and FEAT_PERIPH_MCU bits are set in featurs */ CMD_EXT_CONTROL = 0x12, CMD_GET_EXT_CONTROL_STATUS = 0x13, }; @@ -54,6 +56,7 @@ enum ctl_byte_e {
/* CMD_GET_FEATURES */ enum features_e { + FEAT_PERIPH_MCU = BIT(0), FEAT_EXT_CMDS = BIT(1), };
@@ -84,10 +87,12 @@ static int turris_omnia_mcu_get_function(struct udevice *dev, uint offset) return -EINVAL; return GPIOF_INPUT;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */ + /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL; + if (!(info->features & FEAT_PERIPH_MCU)) + return -EINVAL; return GPIOF_OUTPUT;
default: @@ -120,10 +125,12 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset) return ((((u32)val32[3] << 24) | ((u32)val32[2] << 16) | ((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 0x1;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */ + /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL; + if (!(info->features & FEAT_PERIPH_MCU)) + return -EINVAL; ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2); if (ret) return ret; @@ -162,10 +169,12 @@ static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int valu val16[0] = value ? val16[1] : 0; return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, sizeof(val16));
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */ + /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL; + if (!(info->features & FEAT_PERIPH_MCU)) + return -EINVAL; val32[3] = BIT(offset - 16 - 32) >> 8; val32[2] = BIT(offset - 16 - 32) & 0xff; val32[1] = value ? val32[3] : 0; @@ -282,8 +291,10 @@ static int turris_omnia_mcu_probe(struct udevice *dev)
uc_priv->bank_name = "mcu_";
- if (info->features & FEAT_EXT_CMDS) + if ((info->features & FEAT_EXT_CMDS) && (info->features & FEAT_PERIPH_MCU)) uc_priv->gpio_count = 16 + 32 + 16; + else if (info->features & FEAT_EXT_CMDS) + uc_priv->gpio_count = 16 + 32; else uc_priv->gpio_count = 16;

On 22.09.22 13:25, Pali Rohár wrote:
Currently all GPIOs supported by CMD_EXT_CONTROL/CMD_GET_EXT_CONTROL_STATUS commands (last 16 GPIOs) are available only when FEAT_PERIPH_MCU feature bit is set. So do not register these GPIOs by U-Boot driver when this feature bit is not set, so U-Boot 'gpio' command would see only GPIOs which really exists.
Fixes: 5e4d24ccc115 ("gpio: Add Turris Omnia MCU driver") Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
drivers/gpio/turris_omnia_mcu.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c index 986ccde6bc70..2d2bf2d1dd69 100644 --- a/drivers/gpio/turris_omnia_mcu.c +++ b/drivers/gpio/turris_omnia_mcu.c @@ -16,6 +16,8 @@ enum commands_e {
/* available if FEAT_EXT_CMDS bit is set in features */ CMD_GET_EXT_STATUS_DWORD = 0x11,
- /* available if FEAT_EXT_CMDS and FEAT_PERIPH_MCU bits are set in featurs */ CMD_EXT_CONTROL = 0x12, CMD_GET_EXT_CONTROL_STATUS = 0x13, };
@@ -54,6 +56,7 @@ enum ctl_byte_e {
/* CMD_GET_FEATURES */ enum features_e {
- FEAT_PERIPH_MCU = BIT(0), FEAT_EXT_CMDS = BIT(1), };
@@ -84,10 +87,12 @@ static int turris_omnia_mcu_get_function(struct udevice *dev, uint offset) return -EINVAL; return GPIOF_INPUT;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
/* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL;
return GPIOF_OUTPUT;
default:
@@ -120,10 +125,12 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset) return ((((u32)val32[3] << 24) | ((u32)val32[2] << 16) | ((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 0x1;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2); if (ret) return ret;return -EINVAL;
@@ -162,10 +169,12 @@ static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int valu val16[0] = value ? val16[1] : 0; return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, sizeof(val16));
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
val32[3] = BIT(offset - 16 - 32) >> 8; val32[2] = BIT(offset - 16 - 32) & 0xff; val32[1] = value ? val32[3] : 0;return -EINVAL;
@@ -282,8 +291,10 @@ static int turris_omnia_mcu_probe(struct udevice *dev)
uc_priv->bank_name = "mcu_";
- if (info->features & FEAT_EXT_CMDS)
- if ((info->features & FEAT_EXT_CMDS) && (info->features & FEAT_PERIPH_MCU)) uc_priv->gpio_count = 16 + 32 + 16;
- else if (info->features & FEAT_EXT_CMDS)
else uc_priv->gpio_count = 16;uc_priv->gpio_count = 16 + 32;
Viele Grüße, Stefan Roese

On Thursday 22 September 2022 16:49:32 Stefan Roese wrote:
On 22.09.22 13:25, Pali Rohár wrote:
Currently all GPIOs supported by CMD_EXT_CONTROL/CMD_GET_EXT_CONTROL_STATUS commands (last 16 GPIOs) are available only when FEAT_PERIPH_MCU feature bit is set. So do not register these GPIOs by U-Boot driver when this feature bit is not set, so U-Boot 'gpio' command would see only GPIOs which really exists.
Fixes: 5e4d24ccc115 ("gpio: Add Turris Omnia MCU driver") Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Stefan, will you send this fix into the master branch, so it would be fixed in 2022.10 release?
Thanks, Stefan
drivers/gpio/turris_omnia_mcu.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c index 986ccde6bc70..2d2bf2d1dd69 100644 --- a/drivers/gpio/turris_omnia_mcu.c +++ b/drivers/gpio/turris_omnia_mcu.c @@ -16,6 +16,8 @@ enum commands_e { /* available if FEAT_EXT_CMDS bit is set in features */ CMD_GET_EXT_STATUS_DWORD = 0x11,
- /* available if FEAT_EXT_CMDS and FEAT_PERIPH_MCU bits are set in featurs */ CMD_EXT_CONTROL = 0x12, CMD_GET_EXT_CONTROL_STATUS = 0x13, };
@@ -54,6 +56,7 @@ enum ctl_byte_e { /* CMD_GET_FEATURES */ enum features_e {
- FEAT_PERIPH_MCU = BIT(0), FEAT_EXT_CMDS = BIT(1), };
@@ -84,10 +87,12 @@ static int turris_omnia_mcu_get_function(struct udevice *dev, uint offset) return -EINVAL; return GPIOF_INPUT;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return GPIOF_OUTPUT; default:return -EINVAL;
@@ -120,10 +125,12 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset) return ((((u32)val32[3] << 24) | ((u32)val32[2] << 16) | ((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 0x1;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2); if (ret) return ret;return -EINVAL;
@@ -162,10 +169,12 @@ static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int valu val16[0] = value ? val16[1] : 0; return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, sizeof(val16));
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
val32[3] = BIT(offset - 16 - 32) >> 8; val32[2] = BIT(offset - 16 - 32) & 0xff; val32[1] = value ? val32[3] : 0;return -EINVAL;
@@ -282,8 +291,10 @@ static int turris_omnia_mcu_probe(struct udevice *dev) uc_priv->bank_name = "mcu_";
- if (info->features & FEAT_EXT_CMDS)
- if ((info->features & FEAT_EXT_CMDS) && (info->features & FEAT_PERIPH_MCU)) uc_priv->gpio_count = 16 + 32 + 16;
- else if (info->features & FEAT_EXT_CMDS)
else uc_priv->gpio_count = 16;uc_priv->gpio_count = 16 + 32;
Viele Grüße, Stefan Roese
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

On 24.09.22 20:01, Pali Rohár wrote:
On Thursday 22 September 2022 16:49:32 Stefan Roese wrote:
On 22.09.22 13:25, Pali Rohár wrote:
Currently all GPIOs supported by CMD_EXT_CONTROL/CMD_GET_EXT_CONTROL_STATUS commands (last 16 GPIOs) are available only when FEAT_PERIPH_MCU feature bit is set. So do not register these GPIOs by U-Boot driver when this feature bit is not set, so U-Boot 'gpio' command would see only GPIOs which really exists.
Fixes: 5e4d24ccc115 ("gpio: Add Turris Omnia MCU driver") Signed-off-by: Pali Rohár pali@kernel.org
Reviewed-by: Stefan Roese sr@denx.de
Stefan, will you send this fix into the master branch, so it would be fixed in 2022.10 release?
Will do.
Thanks, Stefan
Thanks, Stefan
drivers/gpio/turris_omnia_mcu.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c index 986ccde6bc70..2d2bf2d1dd69 100644 --- a/drivers/gpio/turris_omnia_mcu.c +++ b/drivers/gpio/turris_omnia_mcu.c @@ -16,6 +16,8 @@ enum commands_e { /* available if FEAT_EXT_CMDS bit is set in features */ CMD_GET_EXT_STATUS_DWORD = 0x11,
- /* available if FEAT_EXT_CMDS and FEAT_PERIPH_MCU bits are set in featurs */ CMD_EXT_CONTROL = 0x12, CMD_GET_EXT_CONTROL_STATUS = 0x13, };
@@ -54,6 +56,7 @@ enum ctl_byte_e { /* CMD_GET_FEATURES */ enum features_e {
- FEAT_PERIPH_MCU = BIT(0), FEAT_EXT_CMDS = BIT(1), };
@@ -84,10 +87,12 @@ static int turris_omnia_mcu_get_function(struct udevice *dev, uint offset) return -EINVAL; return GPIOF_INPUT;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
default:return -EINVAL; return GPIOF_OUTPUT;
@@ -120,10 +125,12 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset) return ((((u32)val32[3] << 24) | ((u32)val32[2] << 16) | ((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 0x1;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL; ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2); if (ret) return ret;
@@ -162,10 +169,12 @@ static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int valu val16[0] = value ? val16[1] : 0; return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, sizeof(val16));
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL; val32[3] = BIT(offset - 16 - 32) >> 8; val32[2] = BIT(offset - 16 - 32) & 0xff; val32[1] = value ? val32[3] : 0;
@@ -282,8 +291,10 @@ static int turris_omnia_mcu_probe(struct udevice *dev) uc_priv->bank_name = "mcu_";
- if (info->features & FEAT_EXT_CMDS)
- if ((info->features & FEAT_EXT_CMDS) && (info->features & FEAT_PERIPH_MCU)) uc_priv->gpio_count = 16 + 32 + 16;
- else if (info->features & FEAT_EXT_CMDS)
else uc_priv->gpio_count = 16;uc_priv->gpio_count = 16 + 32;
Viele Grüße, Stefan Roese
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
Viele Grüße, Stefan Roese

On 22.09.22 13:25, Pali Rohár wrote:
Currently all GPIOs supported by CMD_EXT_CONTROL/CMD_GET_EXT_CONTROL_STATUS commands (last 16 GPIOs) are available only when FEAT_PERIPH_MCU feature bit is set. So do not register these GPIOs by U-Boot driver when this feature bit is not set, so U-Boot 'gpio' command would see only GPIOs which really exists.
Fixes: 5e4d24ccc115 ("gpio: Add Turris Omnia MCU driver") Signed-off-by: Pali Rohár pali@kernel.org
Applied to u-boot-marvell/master
Thanks, Stefan
drivers/gpio/turris_omnia_mcu.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c index 986ccde6bc70..2d2bf2d1dd69 100644 --- a/drivers/gpio/turris_omnia_mcu.c +++ b/drivers/gpio/turris_omnia_mcu.c @@ -16,6 +16,8 @@ enum commands_e {
/* available if FEAT_EXT_CMDS bit is set in features */ CMD_GET_EXT_STATUS_DWORD = 0x11,
- /* available if FEAT_EXT_CMDS and FEAT_PERIPH_MCU bits are set in featurs */ CMD_EXT_CONTROL = 0x12, CMD_GET_EXT_CONTROL_STATUS = 0x13, };
@@ -54,6 +56,7 @@ enum ctl_byte_e {
/* CMD_GET_FEATURES */ enum features_e {
- FEAT_PERIPH_MCU = BIT(0), FEAT_EXT_CMDS = BIT(1), };
@@ -84,10 +87,12 @@ static int turris_omnia_mcu_get_function(struct udevice *dev, uint offset) return -EINVAL; return GPIOF_INPUT;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
/* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL;
return GPIOF_OUTPUT;
default:
@@ -120,10 +125,12 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset) return ((((u32)val32[3] << 24) | ((u32)val32[2] << 16) | ((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 0x1;
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2); if (ret) return ret;return -EINVAL;
@@ -162,10 +169,12 @@ static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int valu val16[0] = value ? val16[1] : 0; return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, sizeof(val16));
- /* bank 2 - supported only when FEAT_EXT_CMDS is set */
- /* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */ case (16 + 32 + 0) ... (16 + 32 + 15): if (!(info->features & FEAT_EXT_CMDS)) return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
val32[3] = BIT(offset - 16 - 32) >> 8; val32[2] = BIT(offset - 16 - 32) & 0xff; val32[1] = value ? val32[3] : 0;return -EINVAL;
@@ -282,8 +291,10 @@ static int turris_omnia_mcu_probe(struct udevice *dev)
uc_priv->bank_name = "mcu_";
- if (info->features & FEAT_EXT_CMDS)
- if ((info->features & FEAT_EXT_CMDS) && (info->features & FEAT_PERIPH_MCU)) uc_priv->gpio_count = 16 + 32 + 16;
- else if (info->features & FEAT_EXT_CMDS)
else uc_priv->gpio_count = 16;uc_priv->gpio_count = 16 + 32;
Viele Grüße, Stefan Roese
participants (2)
-
Pali Rohár
-
Stefan Roese