diff mbox series

phy: mvebu-cp110-utmi: support swapping d+/d- lanes by dts property

Message ID 20240704-mvebu-utmi-phy-v1-1-9d3c8eea46e5@solid-run.com
State Superseded
Headers show
Series phy: mvebu-cp110-utmi: support swapping d+/d- lanes by dts property | expand

Commit Message

Josua Mayer July 4, 2024, 2:57 p.m. UTC 
CP11x UTMI PHY supports swapping D+/D- signals via digital control
register 1.

Add support for the "swap-dx-lanes" device-tree property, which lists
the port-ids that should swap D+ and D-.
The property is evaluated in probe and applied before power-on
during mvebu_cp110_utmi_port_setup.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/phy/marvell/phy-mvebu-cp110-utmi.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)


---
base-commit: 4de480264948cbf2b08a72c39ee122a9f9761e03
change-id: 20240704-mvebu-utmi-phy-84aa3bf3957d

Sincerely,

Comments

Vinod Koul Aug. 4, 2024, 5:48 p.m. UTC | #1 
On 04-07-24, 16:57, Josua Mayer wrote:
> CP11x UTMI PHY supports swapping D+/D- signals via digital control
> register 1.
> 
> Add support for the "swap-dx-lanes" device-tree property, which lists
> the port-ids that should swap D+ and D-.
> The property is evaluated in probe and applied before power-on
> during mvebu_cp110_utmi_port_setup.

It fails my build tests

drivers/phy/marvell/phy-mvebu-cp110-utmi.c:
In function ‘mvebu_cp110_utmi_phy_probe’:
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:89:
error: macro "of_property_for_each_u32" passed 5 arguments, but takes
just 3
  361 |                 of_property_for_each_u32(dev->of_node,
"swap-dx-lanes", prop, p, swap_dx)
      |
^
In file included from
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:15:
include/linux/of.h:1432: note:
macro "of_property_for_each_u32" defined here
 1432 | #define of_property_for_each_u32(np, propname, u)
\
      | 
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:17:
error: ‘of_property_for_each_u32’ undeclared (first use in this
function); did you mean ‘of_property_read_u32’?
  361 |                 of_property_for_each_u32(dev->of_node,
"swap-dx-lanes", prop, p, swap_dx)
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~
      |                 of_property_read_u32
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:17:
note: each undeclared identifier is reported only once for each function
it appears in
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:41:
error: expected ‘;’ before ‘if’
  361 |                 of_property_for_each_u32(dev->of_node,
"swap-dx-lanes", prop, p, swap_dx)
      |                                         ^
      |                                         ;
  362 |                         if (swap_dx == port_id)
      |                         ~~               
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:301:13:
error: unused variable ‘swap_dx’ [-Werror=unused-variable]
  301 |         u32 swap_dx = 0;
      |             ^~~~~~~
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:299:23:
error: unused variable ‘p’ [-Werror=unused-variable]
  299 |         const __be32 *p;
      |                       ^
drivers/phy/marvell/phy-mvebu-cp110-utmi.c:298:26:
error: unused variable ‘prop’ [-Werror=unused-variable]
  298 |         struct property *prop;
      |                          ^~~~
cc1: all warnings being treated as errors


> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  drivers/phy/marvell/phy-mvebu-cp110-utmi.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
> index 4922a5f3327d..5e403f842e9a 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
> @@ -62,6 +62,8 @@
>  #define   SQ_AMP_CAL_MASK			GENMASK(2, 0)
>  #define   SQ_AMP_CAL_VAL			1
>  #define   SQ_AMP_CAL_EN				BIT(3)
> +#define UTMI_DIG_CTRL1_REG			0x20
> +#define   SWAP_DPDM				BIT(15)
>  #define UTMI_CTRL_STATUS0_REG			0x24
>  #define   SUSPENDM				BIT(22)
>  #define   TEST_SEL				BIT(25)
> @@ -104,6 +106,7 @@ struct mvebu_cp110_utmi_port {
>  	struct mvebu_cp110_utmi *priv;
>  	u32 id;
>  	enum usb_dr_mode dr_mode;
> +	bool swap_dx;
>  };
>  
>  static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
> @@ -159,6 +162,13 @@ static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
>  	reg &= ~(VDAT_MASK | VSRC_MASK);
>  	reg |= (VDAT_VAL << VDAT_OFFSET) | (VSRC_VAL << VSRC_OFFSET);
>  	writel(reg, PORT_REGS(port) + UTMI_CHGDTC_CTRL_REG);
> +
> +	/* Swap D+/D- */
> +	reg = readl(PORT_REGS(port) + UTMI_DIG_CTRL1_REG);
> +	reg &= ~(SWAP_DPDM);
> +	if (port->swap_dx)
> +		reg |= SWAP_DPDM;
> +	writel(reg, PORT_REGS(port) + UTMI_DIG_CTRL1_REG);
>  }
>  
>  static int mvebu_cp110_utmi_phy_power_off(struct phy *phy)
> @@ -285,7 +295,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>  	struct mvebu_cp110_utmi *utmi;
>  	struct phy_provider *provider;
>  	struct device_node *child;
> +	struct property *prop;
> +	const __be32 *p;
>  	u32 usb_devices = 0;
> +	u32 swap_dx = 0;
>  
>  	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
>  	if (!utmi)
> @@ -345,6 +358,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>  			}
>  		}
>  
> +		of_property_for_each_u32(dev->of_node, "swap-dx-lanes", prop, p, swap_dx)
> +			if (swap_dx == port_id)
> +				port->swap_dx = 1;
> +
>  		/* Retrieve PHY capabilities */
>  		utmi->ops = &mvebu_cp110_utmi_phy_ops;
>  
> 
> ---
> base-commit: 4de480264948cbf2b08a72c39ee122a9f9761e03
> change-id: 20240704-mvebu-utmi-phy-84aa3bf3957d
> 
> Sincerely,
> -- 
> Josua Mayer <josua@solid-run.com>
Josua Mayer Sept. 3, 2024, 8:04 a.m. UTC | #2 
Hi Vinod,

Thank you for looking at my patch!

Am 04.08.24 um 20:48 schrieb Vinod Koul:
> On 04-07-24, 16:57, Josua Mayer wrote:
>> CP11x UTMI PHY supports swapping D+/D- signals via digital control
>> register 1.
>>
>> Add support for the "swap-dx-lanes" device-tree property, which lists
>> the port-ids that should swap D+ and D-.
>> The property is evaluated in probe and applied before power-on
>> during mvebu_cp110_utmi_port_setup.
> It fails my build tests
I will rebase on v6.11-rc1, compile-test and then submit a new version.
>
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:
> In function ‘mvebu_cp110_utmi_phy_probe’:
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:89:
> error: macro "of_property_for_each_u32" passed 5 arguments, but takes
> just 3
>    361 |                 of_property_for_each_u32(dev->of_node,
> "swap-dx-lanes", prop, p, swap_dx)
>        |
> ^
> In file included from
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:15:
> include/linux/of.h:1432: note:
> macro "of_property_for_each_u32" defined here
>   1432 | #define of_property_for_each_u32(np, propname, u)
> \
>        |
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:17:
> error: ‘of_property_for_each_u32’ undeclared (first use in this
> function); did you mean ‘of_property_read_u32’?
>    361 |                 of_property_for_each_u32(dev->of_node,
> "swap-dx-lanes", prop, p, swap_dx)
>        |                 ^~~~~~~~~~~~~~~~~~~~~~~~
>        |                 of_property_read_u32
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:17:
> note: each undeclared identifier is reported only once for each function
> it appears in
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:361:41:
> error: expected ‘;’ before ‘if’
>    361 |                 of_property_for_each_u32(dev->of_node,
> "swap-dx-lanes", prop, p, swap_dx)
>        |                                         ^
>        |                                         ;
>    362 |                         if (swap_dx == port_id)
>        |                         ~~
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:301:13:
> error: unused variable ‘swap_dx’ [-Werror=unused-variable]
>    301 |         u32 swap_dx = 0;
>        |             ^~~~~~~
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:299:23:
> error: unused variable ‘p’ [-Werror=unused-variable]
>    299 |         const __be32 *p;
>        |                       ^
> drivers/phy/marvell/phy-mvebu-cp110-utmi.c:298:26:
> error: unused variable ‘prop’ [-Werror=unused-variable]
>    298 |         struct property *prop;
>        |                          ^~~~
> cc1: all warnings being treated as errors
>
>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>   drivers/phy/marvell/phy-mvebu-cp110-utmi.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
>> index 4922a5f3327d..5e403f842e9a 100644
>> --- a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
>> +++ b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
>> @@ -62,6 +62,8 @@
>>   #define   SQ_AMP_CAL_MASK			GENMASK(2, 0)
>>   #define   SQ_AMP_CAL_VAL			1
>>   #define   SQ_AMP_CAL_EN				BIT(3)
>> +#define UTMI_DIG_CTRL1_REG			0x20
>> +#define   SWAP_DPDM				BIT(15)
>>   #define UTMI_CTRL_STATUS0_REG			0x24
>>   #define   SUSPENDM				BIT(22)
>>   #define   TEST_SEL				BIT(25)
>> @@ -104,6 +106,7 @@ struct mvebu_cp110_utmi_port {
>>   	struct mvebu_cp110_utmi *priv;
>>   	u32 id;
>>   	enum usb_dr_mode dr_mode;
>> +	bool swap_dx;
>>   };
>>   
>>   static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
>> @@ -159,6 +162,13 @@ static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
>>   	reg &= ~(VDAT_MASK | VSRC_MASK);
>>   	reg |= (VDAT_VAL << VDAT_OFFSET) | (VSRC_VAL << VSRC_OFFSET);
>>   	writel(reg, PORT_REGS(port) + UTMI_CHGDTC_CTRL_REG);
>> +
>> +	/* Swap D+/D- */
>> +	reg = readl(PORT_REGS(port) + UTMI_DIG_CTRL1_REG);
>> +	reg &= ~(SWAP_DPDM);
>> +	if (port->swap_dx)
>> +		reg |= SWAP_DPDM;
>> +	writel(reg, PORT_REGS(port) + UTMI_DIG_CTRL1_REG);
>>   }
>>   
>>   static int mvebu_cp110_utmi_phy_power_off(struct phy *phy)
>> @@ -285,7 +295,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>>   	struct mvebu_cp110_utmi *utmi;
>>   	struct phy_provider *provider;
>>   	struct device_node *child;
>> +	struct property *prop;
>> +	const __be32 *p;
>>   	u32 usb_devices = 0;
>> +	u32 swap_dx = 0;
>>   
>>   	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
>>   	if (!utmi)
>> @@ -345,6 +358,10 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
>>   			}
>>   		}
>>   
>> +		of_property_for_each_u32(dev->of_node, "swap-dx-lanes", prop, p, swap_dx)
>> +			if (swap_dx == port_id)
>> +				port->swap_dx = 1;
>> +
>>   		/* Retrieve PHY capabilities */
>>   		utmi->ops = &mvebu_cp110_utmi_phy_ops;
>>   
>>
>> ---
>> base-commit: 4de480264948cbf2b08a72c39ee122a9f9761e03
>> change-id: 20240704-mvebu-utmi-phy-84aa3bf3957d
>>
>> Sincerely,
>> -- 
>> Josua Mayer <josua@solid-run.com>
Vinod Koul Oct. 7, 2024, 3:48 p.m. UTC | #3 
On Thu, 04 Jul 2024 16:57:43 +0200, Josua Mayer wrote:
> CP11x UTMI PHY supports swapping D+/D- signals via digital control
> register 1.
> 
> Add support for the "swap-dx-lanes" device-tree property, which lists
> the port-ids that should swap D+ and D-.
> The property is evaluated in probe and applied before power-on
> during mvebu_cp110_utmi_port_setup.
> 
> [...]

Applied, thanks!

[1/1] phy: mvebu-cp110-utmi: support swapping d+/d- lanes by dts property
      commit: d6c496f05e98c6d25ac73f23fd0075913620de56

Best regards,
diff mbox series

Patch

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
index 4922a5f3327d..5e403f842e9a 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
@@ -62,6 +62,8 @@ 
 #define   SQ_AMP_CAL_MASK			GENMASK(2, 0)
 #define   SQ_AMP_CAL_VAL			1
 #define   SQ_AMP_CAL_EN				BIT(3)
+#define UTMI_DIG_CTRL1_REG			0x20
+#define   SWAP_DPDM				BIT(15)
 #define UTMI_CTRL_STATUS0_REG			0x24
 #define   SUSPENDM				BIT(22)
 #define   TEST_SEL				BIT(25)
@@ -104,6 +106,7 @@  struct mvebu_cp110_utmi_port {
 	struct mvebu_cp110_utmi *priv;
 	u32 id;
 	enum usb_dr_mode dr_mode;
+	bool swap_dx;
 };
 
 static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
@@ -159,6 +162,13 @@  static void mvebu_cp110_utmi_port_setup(struct mvebu_cp110_utmi_port *port)
 	reg &= ~(VDAT_MASK | VSRC_MASK);
 	reg |= (VDAT_VAL << VDAT_OFFSET) | (VSRC_VAL << VSRC_OFFSET);
 	writel(reg, PORT_REGS(port) + UTMI_CHGDTC_CTRL_REG);
+
+	/* Swap D+/D- */
+	reg = readl(PORT_REGS(port) + UTMI_DIG_CTRL1_REG);
+	reg &= ~(SWAP_DPDM);
+	if (port->swap_dx)
+		reg |= SWAP_DPDM;
+	writel(reg, PORT_REGS(port) + UTMI_DIG_CTRL1_REG);
 }
 
 static int mvebu_cp110_utmi_phy_power_off(struct phy *phy)
@@ -285,7 +295,10 @@  static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 	struct mvebu_cp110_utmi *utmi;
 	struct phy_provider *provider;
 	struct device_node *child;
+	struct property *prop;
+	const __be32 *p;
 	u32 usb_devices = 0;
+	u32 swap_dx = 0;
 
 	utmi = devm_kzalloc(dev, sizeof(*utmi), GFP_KERNEL);
 	if (!utmi)
@@ -345,6 +358,10 @@  static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
 			}
 		}
 
+		of_property_for_each_u32(dev->of_node, "swap-dx-lanes", prop, p, swap_dx)
+			if (swap_dx == port_id)
+				port->swap_dx = 1;
+
 		/* Retrieve PHY capabilities */
 		utmi->ops = &mvebu_cp110_utmi_phy_ops;