Re: [PATCH u-boot-mvebu v3 12/18] gpio: turris_omnia_mcu: Use byteorder conversion functions

2024-03-28 Thread Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Use byteorder conversion function instead of manually assembling data
from/to MCU.

Signed-off-by: Marek Behún 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  drivers/gpio/turris_omnia_mcu.c | 80 +++--
  1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c
index da9a6efe6d..c441d07d69 100644
--- a/drivers/gpio/turris_omnia_mcu.c
+++ b/drivers/gpio/turris_omnia_mcu.c
@@ -5,6 +5,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  
@@ -51,27 +52,31 @@ static int turris_omnia_mcu_get_function(struct udevice *dev, uint offset)

  static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset)
  {
struct turris_omnia_mcu_info *info = dev_get_plat(dev);
-   u8 val16[2];
-   u8 val32[4];
+   u32 val32;
+   u16 val16;
int ret;
  
  	switch (offset) {

/* bank 0 */
case 0 ... 15:
-   ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, val16, 2);
+   ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, (void *),
+ sizeof(val16));
if (ret)
return ret;
-   return u16)val16[1] << 8) | val16[0]) >> offset) & 0x1;
+
+   return !!(le16_to_cpu(val16) & BIT(offset));
  
  	/* bank 1 - supported only when FEAT_EXT_CMDS is set */

case (16 + 0) ... (16 + 31):
if (!(info->features & FEAT_EXT_CMDS))
return -EINVAL;
-   ret = dm_i2c_read(dev, CMD_GET_EXT_STATUS_DWORD, val32, 4);
+
+   ret = dm_i2c_read(dev, CMD_GET_EXT_STATUS_DWORD, (void *),
+ sizeof(val32));
if (ret)
return ret;
-   return u32)val32[3] << 24) | ((u32)val32[2] << 16) |
-((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 
0x1;
+
+   return !!(le32_to_cpu(val32) & BIT(offset - 16));
  
  	/* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */

case (16 + 32 + 0) ... (16 + 32 + 15):
@@ -79,10 +84,13 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, 
uint offset)
return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL;
-   ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2);
+
+   ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS,
+ (void *), sizeof(val16));
if (ret)
return ret;
-   return u16)val16[1] << 8) | val16[0]) >> (offset - 16 - 32)) 
& 0x1;
+
+   return !!(le16_to_cpu(val16) & BIT(offset - 16 - 32));
  
  	default:

return -EINVAL;
@@ -92,30 +100,33 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, 
uint offset)
  static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int 
value)
  {
struct turris_omnia_mcu_info *info = dev_get_plat(dev);
-   u8 val16[2];
-   u8 val32[4];
+   u16 valmask16[2];
+   u8 valmask8[2];
  
  	switch (offset) {

/* bank 0 */
case 0 ... 15:
switch (offset) {
case ilog2(STS_USB30_PWRON):
-   val16[1] = CTL_USB30_PWRON;
+   valmask8[1] = CTL_USB30_PWRON;
break;
case ilog2(STS_USB31_PWRON):
-   val16[1] = CTL_USB31_PWRON;
+   valmask8[1] = CTL_USB31_PWRON;
break;
case ilog2(STS_ENABLE_4V5):
-   val16[1] = CTL_ENABLE_4V5;
+   valmask8[1] = CTL_ENABLE_4V5;
break;
case ilog2(STS_BUTTON_MODE):
-   val16[1] = CTL_BUTTON_MODE;
+   valmask8[1] = CTL_BUTTON_MODE;
break;
default:
return -EINVAL;
}
-   val16[0] = value ? val16[1] : 0;
-   return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, 
sizeof(val16));
+
+   valmask8[0] = value ? valmask8[1] : 0;
+
+   return dm_i2c_write(dev, CMD_GENERAL_CONTROL, valmask8,
+   sizeof(valmask8));
  
  	/* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is set */

case (16 + 32 + 0) ... (16 + 32 + 15):
@@ -123,11 +134,12 @@ static int turris_omnia_mcu_set_value(struct udevice 
*dev, uint offset, int valu
return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL;
-   val32[3] = BIT(offset - 16 - 32) >> 8;
-   val32[2] = BIT(offset - 16 - 32) & 0xff;
- 

[PATCH u-boot-mvebu v3 12/18] gpio: turris_omnia_mcu: Use byteorder conversion functions

2024-03-27 Thread Marek Behún
Use byteorder conversion function instead of manually assembling data
from/to MCU.

Signed-off-by: Marek Behún 
---
 drivers/gpio/turris_omnia_mcu.c | 80 +++--
 1 file changed, 46 insertions(+), 34 deletions(-)

diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c
index da9a6efe6d..c441d07d69 100644
--- a/drivers/gpio/turris_omnia_mcu.c
+++ b/drivers/gpio/turris_omnia_mcu.c
@@ -5,6 +5,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -51,27 +52,31 @@ static int turris_omnia_mcu_get_function(struct udevice 
*dev, uint offset)
 static int turris_omnia_mcu_get_value(struct udevice *dev, uint offset)
 {
struct turris_omnia_mcu_info *info = dev_get_plat(dev);
-   u8 val16[2];
-   u8 val32[4];
+   u32 val32;
+   u16 val16;
int ret;
 
switch (offset) {
/* bank 0 */
case 0 ... 15:
-   ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, val16, 2);
+   ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, (void *),
+ sizeof(val16));
if (ret)
return ret;
-   return u16)val16[1] << 8) | val16[0]) >> offset) & 0x1;
+
+   return !!(le16_to_cpu(val16) & BIT(offset));
 
/* bank 1 - supported only when FEAT_EXT_CMDS is set */
case (16 + 0) ... (16 + 31):
if (!(info->features & FEAT_EXT_CMDS))
return -EINVAL;
-   ret = dm_i2c_read(dev, CMD_GET_EXT_STATUS_DWORD, val32, 4);
+
+   ret = dm_i2c_read(dev, CMD_GET_EXT_STATUS_DWORD, (void *),
+ sizeof(val32));
if (ret)
return ret;
-   return u32)val32[3] << 24) | ((u32)val32[2] << 16) |
-((u32)val32[1] << 8) | val32[0]) >> (offset - 16)) & 
0x1;
+
+   return !!(le32_to_cpu(val32) & BIT(offset - 16));
 
/* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is 
set */
case (16 + 32 + 0) ... (16 + 32 + 15):
@@ -79,10 +84,13 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, 
uint offset)
return -EINVAL;
if (!(info->features & FEAT_PERIPH_MCU))
return -EINVAL;
-   ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS, val16, 2);
+
+   ret = dm_i2c_read(dev, CMD_GET_EXT_CONTROL_STATUS,
+ (void *), sizeof(val16));
if (ret)
return ret;
-   return u16)val16[1] << 8) | val16[0]) >> (offset - 16 - 
32)) & 0x1;
+
+   return !!(le16_to_cpu(val16) & BIT(offset - 16 - 32));
 
default:
return -EINVAL;
@@ -92,30 +100,33 @@ static int turris_omnia_mcu_get_value(struct udevice *dev, 
uint offset)
 static int turris_omnia_mcu_set_value(struct udevice *dev, uint offset, int 
value)
 {
struct turris_omnia_mcu_info *info = dev_get_plat(dev);
-   u8 val16[2];
-   u8 val32[4];
+   u16 valmask16[2];
+   u8 valmask8[2];
 
switch (offset) {
/* bank 0 */
case 0 ... 15:
switch (offset) {
case ilog2(STS_USB30_PWRON):
-   val16[1] = CTL_USB30_PWRON;
+   valmask8[1] = CTL_USB30_PWRON;
break;
case ilog2(STS_USB31_PWRON):
-   val16[1] = CTL_USB31_PWRON;
+   valmask8[1] = CTL_USB31_PWRON;
break;
case ilog2(STS_ENABLE_4V5):
-   val16[1] = CTL_ENABLE_4V5;
+   valmask8[1] = CTL_ENABLE_4V5;
break;
case ilog2(STS_BUTTON_MODE):
-   val16[1] = CTL_BUTTON_MODE;
+   valmask8[1] = CTL_BUTTON_MODE;
break;
default:
return -EINVAL;
}
-   val16[0] = value ? val16[1] : 0;
-   return dm_i2c_write(dev, CMD_GENERAL_CONTROL, val16, 
sizeof(val16));
+
+   valmask8[0] = value ? valmask8[1] : 0;
+
+   return dm_i2c_write(dev, CMD_GENERAL_CONTROL, valmask8,
+   sizeof(valmask8));
 
/* bank 2 - supported only when FEAT_EXT_CMDS and FEAT_PERIPH_MCU is 
set */
case (16 + 32 + 0) ... (16 + 32 + 15):
@@ -123,11 +134,12 @@ static int turris_omnia_mcu_set_value(struct udevice 
*dev, uint offset, int valu
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;
-   val32[0] = value ?