diff mbox series

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

Message ID 20240327162355.24584-13-kabel@kernel.org
State Superseded
Delegated to: Stefan Roese
Headers show
Series Turris Omnia - New board revision support | expand

Commit Message

Marek Behún March 27, 2024, 4:23 p.m. UTC
Use byteorder conversion function instead of manually assembling data
from/to MCU.

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

Comments

Stefan Roese March 28, 2024, 10:12 a.m. UTC | #1
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 <kabel@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

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 <dm.h>
>   #include <i2c.h>
>   #include <turris-omnia-mcu-interface.h>
> +#include <asm/byteorder.h>
>   #include <asm/gpio.h>
>   #include <linux/log2.h>
>   
> @@ -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 *)&val16,
> +				  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 *)&val32,
> +				  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 *)&val16, 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 ? val32[2] : 0;
> -		return dm_i2c_write(dev, CMD_EXT_CONTROL, val32, sizeof(val32));
> +
> +		valmask16[1] = cpu_to_le16(BIT(offset - 16 - 32));
> +		valmask16[0] = value ? valmask16[1] : 0;
> +
> +		return dm_i2c_write(dev, CMD_EXT_CONTROL, (void *)valmask16,
> +				    sizeof(valmask16));
>   
>   	default:
>   		return -EINVAL;
> @@ -216,25 +228,25 @@ static int turris_omnia_mcu_probe(struct udevice *dev)
>   {
>   	struct turris_omnia_mcu_info *info = dev_get_plat(dev);
>   	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> -	u16 status;
> -	u8 val[2];
> +	u16 val;
>   	int ret;
>   
> -	ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, val, 2);
> -	if (ret) {
> -		printf("Error: turris_omnia_mcu CMD_GET_STATUS_WORD failed: %d\n", ret);
> +	ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, (void *)&val, sizeof(val));
> +	if (ret < 0) {
> +		printf("Error: turris_omnia_mcu CMD_GET_STATUS_WORD failed: %d\n",
> +		       ret);
>   		return ret;
>   	}
>   
> -	status = ((u16)val[1] << 8) | val[0];
> -
> -	if (status & STS_FEATURES_SUPPORTED) {
> -		ret = dm_i2c_read(dev, CMD_GET_FEATURES, val, 2);
> -		if (ret) {
> -			printf("Error: turris_omnia_mcu CMD_GET_FEATURES failed: %d\n", ret);
> +	if (le16_to_cpu(val) & STS_FEATURES_SUPPORTED) {
> +		ret = dm_i2c_read(dev, CMD_GET_FEATURES, (void *)&val,
> +				  sizeof(val));
> +		if (ret < 0) {
> +			printf("Error: turris_omnia_mcu CMD_GET_FEATURES failed: %d\n",
> +			       ret);
>   			return ret;
>   		}
> -		info->features = ((u16)val[1] << 8) | val[0];
> +		info->features = le16_to_cpu(val);
>   	}
>   
>   	uc_priv->bank_name = "mcu_";

Viele Grüße,
Stefan Roese
diff mbox series

Patch

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 <dm.h>
 #include <i2c.h>
 #include <turris-omnia-mcu-interface.h>
+#include <asm/byteorder.h>
 #include <asm/gpio.h>
 #include <linux/log2.h>
 
@@ -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 *)&val16,
+				  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 *)&val32,
+				  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 *)&val16, 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 ? val32[2] : 0;
-		return dm_i2c_write(dev, CMD_EXT_CONTROL, val32, sizeof(val32));
+
+		valmask16[1] = cpu_to_le16(BIT(offset - 16 - 32));
+		valmask16[0] = value ? valmask16[1] : 0;
+
+		return dm_i2c_write(dev, CMD_EXT_CONTROL, (void *)valmask16,
+				    sizeof(valmask16));
 
 	default:
 		return -EINVAL;
@@ -216,25 +228,25 @@  static int turris_omnia_mcu_probe(struct udevice *dev)
 {
 	struct turris_omnia_mcu_info *info = dev_get_plat(dev);
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
-	u16 status;
-	u8 val[2];
+	u16 val;
 	int ret;
 
-	ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, val, 2);
-	if (ret) {
-		printf("Error: turris_omnia_mcu CMD_GET_STATUS_WORD failed: %d\n", ret);
+	ret = dm_i2c_read(dev, CMD_GET_STATUS_WORD, (void *)&val, sizeof(val));
+	if (ret < 0) {
+		printf("Error: turris_omnia_mcu CMD_GET_STATUS_WORD failed: %d\n",
+		       ret);
 		return ret;
 	}
 
-	status = ((u16)val[1] << 8) | val[0];
-
-	if (status & STS_FEATURES_SUPPORTED) {
-		ret = dm_i2c_read(dev, CMD_GET_FEATURES, val, 2);
-		if (ret) {
-			printf("Error: turris_omnia_mcu CMD_GET_FEATURES failed: %d\n", ret);
+	if (le16_to_cpu(val) & STS_FEATURES_SUPPORTED) {
+		ret = dm_i2c_read(dev, CMD_GET_FEATURES, (void *)&val,
+				  sizeof(val));
+		if (ret < 0) {
+			printf("Error: turris_omnia_mcu CMD_GET_FEATURES failed: %d\n",
+			       ret);
 			return ret;
 		}
-		info->features = ((u16)val[1] << 8) | val[0];
+		info->features = le16_to_cpu(val);
 	}
 
 	uc_priv->bank_name = "mcu_";