* [net-next PATCH RESEND 0/2] PolarFire SoC macb reset support
@ 2022-07-01 6:58 Conor Dooley
2022-07-01 6:58 ` [net-next PATCH RESEND 1/2] dt-bindings: net: cdns,macb: document polarfire soc's macb Conor Dooley
2022-07-01 6:58 ` [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support Conor Dooley
0 siblings, 2 replies; 10+ messages in thread
From: Conor Dooley @ 2022-07-01 6:58 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Nicolas Ferre, Claudiu Beznea
Cc: netdev, devicetree, linux-riscv, linux-kernel, Conor Dooley
Hey all,
Jakub requested that these patches be split off from the series
adding the reset controller itself that I sent yesterday [0].
The Cadence MACBs on PolarFire SoC (MPFS) have reset capability and are
compatible with the zynqmp's init function. I have removed the zynqmp
specific comments from that function & renamed it to reflect what it
does, since it is no longer zynqmp only.
MPFS's MACB had previously used the generic binding, so I also added
the required specific binding.
Thanks,
Conor.
0 - https://lore.kernel.org/all/20220630080532.323731-1-conor.dooley@microchip.com/
Conor Dooley (2):
dt-bindings: net: cdns,macb: document polarfire soc's macb
net: macb: add polarfire soc reset support
.../devicetree/bindings/net/cdns,macb.yaml | 1 +
drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++------
2 files changed, 19 insertions(+), 7 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [net-next PATCH RESEND 1/2] dt-bindings: net: cdns,macb: document polarfire soc's macb
2022-07-01 6:58 [net-next PATCH RESEND 0/2] PolarFire SoC macb reset support Conor Dooley
@ 2022-07-01 6:58 ` Conor Dooley
2022-07-01 20:35 ` Rob Herring
2022-07-01 6:58 ` [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support Conor Dooley
1 sibling, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2022-07-01 6:58 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Nicolas Ferre, Claudiu Beznea
Cc: netdev, devicetree, linux-riscv, linux-kernel, Conor Dooley
Until now the PolarFire SoC (MPFS) has been using the generic
"cdns,macb" compatible but has optional reset support. Add a specific
compatible which falls back to the currently used generic binding.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
Documentation/devicetree/bindings/net/cdns,macb.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index 86fc31c2d91b..9c92156869b2 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -28,6 +28,7 @@ properties:
- enum:
- cdns,at91sam9260-macb # Atmel at91sam9 SoCs
- cdns,sam9x60-macb # Microchip sam9x60 SoC
+ - microchip,mpfs-macb # Microchip PolarFire SoC
- const: cdns,macb # Generic
- items:
--
2.36.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-01 6:58 [net-next PATCH RESEND 0/2] PolarFire SoC macb reset support Conor Dooley
2022-07-01 6:58 ` [net-next PATCH RESEND 1/2] dt-bindings: net: cdns,macb: document polarfire soc's macb Conor Dooley
@ 2022-07-01 6:58 ` Conor Dooley
2022-07-01 7:47 ` Claudiu.Beznea
1 sibling, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2022-07-01 6:58 UTC (permalink / raw)
To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Krzysztof Kozlowski, Nicolas Ferre, Claudiu Beznea
Cc: netdev, devicetree, linux-riscv, linux-kernel, Conor Dooley
To date, the Microchip PolarFire SoC (MPFS) has been using the
cdns,macb compatible, however the generic device does not have reset
support. Add a new compatible & .data for MPFS to hook into the reset
functionality added for zynqmp support (and make the zynqmp init
function generic in the process).
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d89098f4ede8..325f0463fd42 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
.usrio = &macb_default_usrio,
};
-static int zynqmp_init(struct platform_device *pdev)
+static int init_reset_optional(struct platform_device *pdev)
{
struct net_device *dev = platform_get_drvdata(pdev);
struct macb *bp = netdev_priv(dev);
int ret;
if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
- /* Ensure PS-GTR PHY device used in SGMII mode is ready */
+ /* Ensure PHY device used in SGMII mode is ready */
bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
if (IS_ERR(bp->sgmii_phy)) {
ret = PTR_ERR(bp->sgmii_phy);
dev_err_probe(&pdev->dev, ret,
- "failed to get PS-GTR PHY\n");
+ "failed to get SGMII PHY\n");
return ret;
}
ret = phy_init(bp->sgmii_phy);
if (ret) {
- dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
+ dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
ret);
return ret;
}
}
- /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
- * if mapped in device tree.
+ /* Fully reset controller at hardware level if mapped in device tree
*/
ret = device_reset_optional(&pdev->dev);
if (ret) {
@@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
- .init = zynqmp_init,
+ .init = init_reset_optional,
.jumbo_max_len = 10240,
.usrio = &macb_default_usrio,
};
@@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
.usrio = &macb_default_usrio,
};
+static const struct macb_config mpfs_config = {
+ .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
+ MACB_CAPS_JUMBO |
+ MACB_CAPS_GEM_HAS_PTP,
+ .dma_burst_length = 16,
+ .clk_init = macb_clk_init,
+ .init = init_reset_optional,
+ .usrio = &macb_default_usrio,
+ .jumbo_max_len = 10240,
+};
+
static const struct macb_config sama7g5_gem_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_CLK_HW_CHG |
MACB_CAPS_MIIONRGMII,
@@ -4787,6 +4797,7 @@ static const struct of_device_id macb_dt_ids[] = {
{ .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config},
{ .compatible = "cdns,zynq-gem", .data = &zynq_config },
{ .compatible = "sifive,fu540-c000-gem", .data = &fu540_c000_config },
+ { .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
{ .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
{ .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
{ /* sentinel */ }
--
2.36.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-01 6:58 ` [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support Conor Dooley
@ 2022-07-01 7:47 ` Claudiu.Beznea
2022-07-01 7:55 ` Conor.Dooley
0 siblings, 1 reply; 10+ messages in thread
From: Claudiu.Beznea @ 2022-07-01 7:47 UTC (permalink / raw)
To: Conor.Dooley, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, Nicolas.Ferre
Cc: netdev, devicetree, linux-riscv, linux-kernel
On 01.07.2022 09:58, Conor Dooley wrote:
> To date, the Microchip PolarFire SoC (MPFS) has been using the
> cdns,macb compatible, however the generic device does not have reset
> support. Add a new compatible & .data for MPFS to hook into the reset
> functionality added for zynqmp support (and make the zynqmp init
> function generic in the process).
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index d89098f4ede8..325f0463fd42 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
> .usrio = &macb_default_usrio,
> };
>
> -static int zynqmp_init(struct platform_device *pdev)
> +static int init_reset_optional(struct platform_device *pdev)
It doesn't sound like a good name for this function but I don't have
something better to propose.
> {
> struct net_device *dev = platform_get_drvdata(pdev);
> struct macb *bp = netdev_priv(dev);
> int ret;
>
> if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> - /* Ensure PS-GTR PHY device used in SGMII mode is ready */
> + /* Ensure PHY device used in SGMII mode is ready */
> bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
>
> if (IS_ERR(bp->sgmii_phy)) {
> ret = PTR_ERR(bp->sgmii_phy);
> dev_err_probe(&pdev->dev, ret,
> - "failed to get PS-GTR PHY\n");
> + "failed to get SGMII PHY\n");
> return ret;
> }
>
> ret = phy_init(bp->sgmii_phy);
> if (ret) {
> - dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
> + dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
> ret);
> return ret;
> }
> }
>
> - /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
> - * if mapped in device tree.
> + /* Fully reset controller at hardware level if mapped in device tree
> */
The new comment can fit on a single line.
> ret = device_reset_optional(&pdev->dev);
> if (ret) {
> @@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
> MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
> .dma_burst_length = 16,
> .clk_init = macb_clk_init,
> - .init = zynqmp_init,
> + .init = init_reset_optional,
> .jumbo_max_len = 10240,
> .usrio = &macb_default_usrio,
> };
> @@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
> .usrio = &macb_default_usrio,
> };
>
> +static const struct macb_config mpfs_config = {
> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
> + MACB_CAPS_JUMBO |
> + MACB_CAPS_GEM_HAS_PTP,
Except for zynqmp and default_gem_config the rest of the capabilities for
other SoCs are aligned something like this:
+ .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
+ MACB_CAPS_JUMBO |
+ MACB_CAPS_GEM_HAS_PTP,
To me it looks better to have you caps aligned this way.
Thank you,
Claudiu Beznea
> + .dma_burst_length = 16,
> + .clk_init = macb_clk_init,
> + .init = init_reset_optional,
> + .usrio = &macb_default_usrio,
> + .jumbo_max_len = 10240,
> +};
> +
> static const struct macb_config sama7g5_gem_config = {
> .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_CLK_HW_CHG |
> MACB_CAPS_MIIONRGMII,
> @@ -4787,6 +4797,7 @@ static const struct of_device_id macb_dt_ids[] = {
> { .compatible = "cdns,zynqmp-gem", .data = &zynqmp_config},
> { .compatible = "cdns,zynq-gem", .data = &zynq_config },
> { .compatible = "sifive,fu540-c000-gem", .data = &fu540_c000_config },
> + { .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
> { .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
> { .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
> { /* sentinel */ }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-01 7:47 ` Claudiu.Beznea
@ 2022-07-01 7:55 ` Conor.Dooley
2022-07-01 8:06 ` Conor.Dooley
2022-07-02 15:34 ` Conor.Dooley
0 siblings, 2 replies; 10+ messages in thread
From: Conor.Dooley @ 2022-07-01 7:55 UTC (permalink / raw)
To: Claudiu.Beznea, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, Nicolas.Ferre
Cc: netdev, devicetree, linux-riscv, linux-kernel
On 01/07/2022 08:47, Claudiu Beznea - M18063 wrote:
> On 01.07.2022 09:58, Conor Dooley wrote:
>> To date, the Microchip PolarFire SoC (MPFS) has been using the
>> cdns,macb compatible, however the generic device does not have reset
>> support. Add a new compatible & .data for MPFS to hook into the reset
>> functionality added for zynqmp support (and make the zynqmp init
>> function generic in the process).
>>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index d89098f4ede8..325f0463fd42 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
>> .usrio = &macb_default_usrio,
>> };
>>
>> -static int zynqmp_init(struct platform_device *pdev)
>> +static int init_reset_optional(struct platform_device *pdev)
>
> It doesn't sound like a good name for this function but I don't have
> something better to propose.
It's better than zynqmp_init, but yeah...
>
>> {
>> struct net_device *dev = platform_get_drvdata(pdev);
>> struct macb *bp = netdev_priv(dev);
>> int ret;
>>
>> if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>> - /* Ensure PS-GTR PHY device used in SGMII mode is ready */
>> + /* Ensure PHY device used in SGMII mode is ready */
>> bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
>>
>> if (IS_ERR(bp->sgmii_phy)) {
>> ret = PTR_ERR(bp->sgmii_phy);
>> dev_err_probe(&pdev->dev, ret,
>> - "failed to get PS-GTR PHY\n");
>> + "failed to get SGMII PHY\n");
>> return ret;
>> }
>>
>> ret = phy_init(bp->sgmii_phy);
>> if (ret) {
>> - dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
>> + dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
>> ret);
>> return ret;
>> }
>> }
>>
>> - /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
>> - * if mapped in device tree.
>> + /* Fully reset controller at hardware level if mapped in device tree
>> */
>
> The new comment can fit on a single line.
>
>> ret = device_reset_optional(&pdev->dev);
>> if (ret) {
>> @@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
>> MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>> .dma_burst_length = 16,
>> .clk_init = macb_clk_init,
>> - .init = zynqmp_init,
>> + .init = init_reset_optional,
>> .jumbo_max_len = 10240,
>> .usrio = &macb_default_usrio,
>> };
>> @@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
>> .usrio = &macb_default_usrio,
>> };
>>
>> +static const struct macb_config mpfs_config = {
>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>> + MACB_CAPS_JUMBO |
>> + MACB_CAPS_GEM_HAS_PTP,
>
> Except for zynqmp and default_gem_config the rest of the capabilities for
> other SoCs are aligned something like this:
>
> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
> + MACB_CAPS_JUMBO |
> + MACB_CAPS_GEM_HAS_PTP,
>
> To me it looks better to have you caps aligned this way.
Yeah, I picked that b/c I copied straight from the default config.
I have no preference, but if you're not a fan of the default...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-01 7:55 ` Conor.Dooley
@ 2022-07-01 8:06 ` Conor.Dooley
2022-07-02 15:34 ` Conor.Dooley
1 sibling, 0 replies; 10+ messages in thread
From: Conor.Dooley @ 2022-07-01 8:06 UTC (permalink / raw)
To: Claudiu.Beznea, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, Nicolas.Ferre
Cc: netdev, devicetree, linux-riscv, linux-kernel
On 01/07/2022 08:55, Conor Dooley wrote:
> On 01/07/2022 08:47, Claudiu Beznea - M18063 wrote:
>> On 01.07.2022 09:58, Conor Dooley wrote:
>>> To date, the Microchip PolarFire SoC (MPFS) has been using the
>>> cdns,macb compatible, however the generic device does not have reset
>>> support. Add a new compatible & .data for MPFS to hook into the reset
>>> functionality added for zynqmp support (and make the zynqmp init
>>> function generic in the process).
>>>
>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>> ---
>>> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
>>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>> index d89098f4ede8..325f0463fd42 100644
>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
>>> .usrio = &macb_default_usrio,
>>> };
>>> -static int zynqmp_init(struct platform_device *pdev)
>>> +static int init_reset_optional(struct platform_device *pdev)
>>
>> It doesn't sound like a good name for this function but I don't have
>> something better to propose.
>
> It's better than zynqmp_init, but yeah...
>
>>
>>> {
>>> struct net_device *dev = platform_get_drvdata(pdev);
>>> struct macb *bp = netdev_priv(dev);
>>> int ret;
>>> if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>>> - /* Ensure PS-GTR PHY device used in SGMII mode is ready */
>>> + /* Ensure PHY device used in SGMII mode is ready */
>>> bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
>>> if (IS_ERR(bp->sgmii_phy)) {
>>> ret = PTR_ERR(bp->sgmii_phy);
>>> dev_err_probe(&pdev->dev, ret,
>>> - "failed to get PS-GTR PHY\n");
>>> + "failed to get SGMII PHY\n");
>>> return ret;
>>> }
>>> ret = phy_init(bp->sgmii_phy);
>>> if (ret) {
>>> - dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
>>> + dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
>>> ret);
>>> return ret;
>>> }
>>> }
>>> - /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
>>> - * if mapped in device tree.
>>> + /* Fully reset controller at hardware level if mapped in device tree
>>> */
>>
>> The new comment can fit on a single line.
>>
>>> ret = device_reset_optional(&pdev->dev);
>>> if (ret) {
>>> @@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
>>> MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>>> .dma_burst_length = 16,
>>> .clk_init = macb_clk_init,
>>> - .init = zynqmp_init,
>>> + .init = init_reset_optional,
>>> .jumbo_max_len = 10240,
>>> .usrio = &macb_default_usrio,
>>> };
>>> @@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
>>> .usrio = &macb_default_usrio,
>>> };
>>> +static const struct macb_config mpfs_config = {
>>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>>> + MACB_CAPS_JUMBO |
>>> + MACB_CAPS_GEM_HAS_PTP,
>>
>> Except for zynqmp and default_gem_config the rest of the capabilities for
>> other SoCs are aligned something like this:
>>
>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>> + MACB_CAPS_JUMBO |
>> + MACB_CAPS_GEM_HAS_PTP,
>>
>> To me it looks better to have you caps aligned this way.
>
> Yeah, I picked that b/c I copied straight from the default config.
> I have no preference, but if you're not a fan of the default...
s/default.../default I can make them all match...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 1/2] dt-bindings: net: cdns,macb: document polarfire soc's macb
2022-07-01 6:58 ` [net-next PATCH RESEND 1/2] dt-bindings: net: cdns,macb: document polarfire soc's macb Conor Dooley
@ 2022-07-01 20:35 ` Rob Herring
0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2022-07-01 20:35 UTC (permalink / raw)
To: Conor Dooley
Cc: David S . Miller, Rob Herring, Krzysztof Kozlowski, devicetree,
Eric Dumazet, linux-kernel, Jakub Kicinski, netdev,
Nicolas Ferre, linux-riscv, Claudiu Beznea, Paolo Abeni
On Fri, 01 Jul 2022 07:58:31 +0100, Conor Dooley wrote:
> Until now the PolarFire SoC (MPFS) has been using the generic
> "cdns,macb" compatible but has optional reset support. Add a specific
> compatible which falls back to the currently used generic binding.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Documentation/devicetree/bindings/net/cdns,macb.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-01 7:55 ` Conor.Dooley
2022-07-01 8:06 ` Conor.Dooley
@ 2022-07-02 15:34 ` Conor.Dooley
2022-07-04 7:07 ` Claudiu.Beznea
1 sibling, 1 reply; 10+ messages in thread
From: Conor.Dooley @ 2022-07-02 15:34 UTC (permalink / raw)
To: Claudiu.Beznea, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, Nicolas.Ferre
Cc: netdev, devicetree, linux-riscv, linux-kernel
On 01/07/2022 08:55, Conor Dooley wrote:
> On 01/07/2022 08:47, Claudiu Beznea - M18063 wrote:
>> On 01.07.2022 09:58, Conor Dooley wrote:
>>> To date, the Microchip PolarFire SoC (MPFS) has been using the
>>> cdns,macb compatible, however the generic device does not have reset
>>> support. Add a new compatible & .data for MPFS to hook into the reset
>>> functionality added for zynqmp support (and make the zynqmp init
>>> function generic in the process).
>>>
>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>> ---
>>> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
>>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>> index d89098f4ede8..325f0463fd42 100644
>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
>>> .usrio = &macb_default_usrio,
>>> };
>>> -static int zynqmp_init(struct platform_device *pdev)
I noticed that this function is oddly placed within the macb_config
structs definitions. Since I am already modifying it, would you like
me to move it above them to where the fu540 init functions are?
>>> +static int init_reset_optional(struct platform_device *pdev)
>>
>> It doesn't sound like a good name for this function but I don't have
>> something better to propose.
>
> It's better than zynqmp_init, but yeah...
>
>>
>>> {
>>> struct net_device *dev = platform_get_drvdata(pdev);
>>> struct macb *bp = netdev_priv(dev);
>>> int ret;
>>> if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>>> - /* Ensure PS-GTR PHY device used in SGMII mode is ready */
>>> + /* Ensure PHY device used in SGMII mode is ready */
>>> bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
>>> if (IS_ERR(bp->sgmii_phy)) {
>>> ret = PTR_ERR(bp->sgmii_phy);
>>> dev_err_probe(&pdev->dev, ret,
>>> - "failed to get PS-GTR PHY\n");
>>> + "failed to get SGMII PHY\n");
>>> return ret;
>>> }
>>> ret = phy_init(bp->sgmii_phy);
>>> if (ret) {
>>> - dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
>>> + dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
>>> ret);
>>> return ret;
>>> }
>>> }
>>> - /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
>>> - * if mapped in device tree.
>>> + /* Fully reset controller at hardware level if mapped in device tree
>>> */
>>
>> The new comment can fit on a single line.
>>
>>> ret = device_reset_optional(&pdev->dev);
>>> if (ret) {
>>> @@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
>>> MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>>> .dma_burst_length = 16,
>>> .clk_init = macb_clk_init,
>>> - .init = zynqmp_init,
>>> + .init = init_reset_optional,
>>> .jumbo_max_len = 10240,
>>> .usrio = &macb_default_usrio,
>>> };
>>> @@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
>>> .usrio = &macb_default_usrio,
>>> };
>>> +static const struct macb_config mpfs_config = {
>>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>>> + MACB_CAPS_JUMBO |
>>> + MACB_CAPS_GEM_HAS_PTP,
>>
>> Except for zynqmp and default_gem_config the rest of the capabilities for
>> other SoCs are aligned something like this:
>>
>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>> + MACB_CAPS_JUMBO |
>> + MACB_CAPS_GEM_HAS_PTP,
>>
>> To me it looks better to have you caps aligned this way.
>
> Yeah, I picked that b/c I copied straight from the default config.
> I have no preference, but if you're not a fan of the default...
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-02 15:34 ` Conor.Dooley
@ 2022-07-04 7:07 ` Claudiu.Beznea
2022-07-04 7:31 ` Conor.Dooley
0 siblings, 1 reply; 10+ messages in thread
From: Claudiu.Beznea @ 2022-07-04 7:07 UTC (permalink / raw)
To: Conor.Dooley, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, Nicolas.Ferre
Cc: netdev, devicetree, linux-riscv, linux-kernel
On 02.07.2022 18:34, Conor Dooley - M52691 wrote:
> On 01/07/2022 08:55, Conor Dooley wrote:
>> On 01/07/2022 08:47, Claudiu Beznea - M18063 wrote:
>>> On 01.07.2022 09:58, Conor Dooley wrote:
>>>> To date, the Microchip PolarFire SoC (MPFS) has been using the
>>>> cdns,macb compatible, however the generic device does not have reset
>>>> support. Add a new compatible & .data for MPFS to hook into the reset
>>>> functionality added for zynqmp support (and make the zynqmp init
>>>> function generic in the process).
>>>>
>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>> ---
>>>> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
>>>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>>> index d89098f4ede8..325f0463fd42 100644
>>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>>> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
>>>> .usrio = &macb_default_usrio,
>>>> };
>>>> -static int zynqmp_init(struct platform_device *pdev)
>
> I noticed that this function is oddly placed within the macb_config
> structs definitions. Since I am already modifying it, would you like
> me to move it above them to where the fu540 init functions are?
That would be good, thanks!
>
>>>> +static int init_reset_optional(struct platform_device *pdev)
>>>
>>> It doesn't sound like a good name for this function but I don't have
>>> something better to propose.
>>
>> It's better than zynqmp_init, but yeah...
>>
>>>
>>>> {
>>>> struct net_device *dev = platform_get_drvdata(pdev);
>>>> struct macb *bp = netdev_priv(dev);
>>>> int ret;
>>>> if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>>>> - /* Ensure PS-GTR PHY device used in SGMII mode is ready */
>>>> + /* Ensure PHY device used in SGMII mode is ready */
>>>> bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
>>>> if (IS_ERR(bp->sgmii_phy)) {
>>>> ret = PTR_ERR(bp->sgmii_phy);
>>>> dev_err_probe(&pdev->dev, ret,
>>>> - "failed to get PS-GTR PHY\n");
>>>> + "failed to get SGMII PHY\n");
>>>> return ret;
>>>> }
>>>> ret = phy_init(bp->sgmii_phy);
>>>> if (ret) {
>>>> - dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
>>>> + dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
>>>> ret);
>>>> return ret;
>>>> }
>>>> }
>>>> - /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
>>>> - * if mapped in device tree.
>>>> + /* Fully reset controller at hardware level if mapped in device tree
>>>> */
>>>
>>> The new comment can fit on a single line.
>>>
>>>> ret = device_reset_optional(&pdev->dev);
>>>> if (ret) {
>>>> @@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
>>>> MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>>>> .dma_burst_length = 16,
>>>> .clk_init = macb_clk_init,
>>>> - .init = zynqmp_init,
>>>> + .init = init_reset_optional,
>>>> .jumbo_max_len = 10240,
>>>> .usrio = &macb_default_usrio,
>>>> };
>>>> @@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
>>>> .usrio = &macb_default_usrio,
>>>> };
>>>> +static const struct macb_config mpfs_config = {
>>>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>>>> + MACB_CAPS_JUMBO |
>>>> + MACB_CAPS_GEM_HAS_PTP,
>>>
>>> Except for zynqmp and default_gem_config the rest of the capabilities for
>>> other SoCs are aligned something like this:
>>>
>>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>>> + MACB_CAPS_JUMBO |
>>> + MACB_CAPS_GEM_HAS_PTP,
>>>
>>> To me it looks better to have you caps aligned this way.
>>
>> Yeah, I picked that b/c I copied straight from the default config.
>> I have no preference, but if you're not a fan of the default...
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support
2022-07-04 7:07 ` Claudiu.Beznea
@ 2022-07-04 7:31 ` Conor.Dooley
0 siblings, 0 replies; 10+ messages in thread
From: Conor.Dooley @ 2022-07-04 7:31 UTC (permalink / raw)
To: Claudiu.Beznea, davem, edumazet, kuba, pabeni, robh+dt,
krzysztof.kozlowski+dt, Nicolas.Ferre
Cc: netdev, devicetree, linux-riscv, linux-kernel
On 04/07/2022 08:07, Claudiu Beznea - M18063 wrote:
> On 02.07.2022 18:34, Conor Dooley - M52691 wrote:
>> On 01/07/2022 08:55, Conor Dooley wrote:
>>> On 01/07/2022 08:47, Claudiu Beznea - M18063 wrote:
>>>> On 01.07.2022 09:58, Conor Dooley wrote:
>>>>> To date, the Microchip PolarFire SoC (MPFS) has been using the
>>>>> cdns,macb compatible, however the generic device does not have reset
>>>>> support. Add a new compatible & .data for MPFS to hook into the reset
>>>>> functionality added for zynqmp support (and make the zynqmp init
>>>>> function generic in the process).
>>>>>
>>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>>> ---
>>>>> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
>>>>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>>>>> index d89098f4ede8..325f0463fd42 100644
>>>>> --- a/drivers/net/ethernet/cadence/macb_main.c
>>>>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>>>>> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
>>>>> .usrio = &macb_default_usrio,
>>>>> };
>>>>> -static int zynqmp_init(struct platform_device *pdev)
>>
>> I noticed that this function is oddly placed within the macb_config
>> structs definitions. Since I am already modifying it, would you like
>> me to move it above them to where the fu540 init functions are?
>
> That would be good, thanks!
Cool, I'll respin with extra patches for the cleanup.
Thanks for the review Claudiu :)
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-07-04 7:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-01 6:58 [net-next PATCH RESEND 0/2] PolarFire SoC macb reset support Conor Dooley
2022-07-01 6:58 ` [net-next PATCH RESEND 1/2] dt-bindings: net: cdns,macb: document polarfire soc's macb Conor Dooley
2022-07-01 20:35 ` Rob Herring
2022-07-01 6:58 ` [net-next PATCH RESEND 2/2] net: macb: add polarfire soc reset support Conor Dooley
2022-07-01 7:47 ` Claudiu.Beznea
2022-07-01 7:55 ` Conor.Dooley
2022-07-01 8:06 ` Conor.Dooley
2022-07-02 15:34 ` Conor.Dooley
2022-07-04 7:07 ` Claudiu.Beznea
2022-07-04 7:31 ` Conor.Dooley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).