mbox series

[0/7] net: macb: add support for sama7g5

Message ID 1607085261-25255-1-git-send-email-claudiu.beznea@microchip.com
Headers show
Series net: macb: add support for sama7g5 | expand

Message

Claudiu Beznea Dec. 4, 2020, 12:34 p.m. UTC
Hi,

This series adds support for SAMA7G5 Ethernet interfaces: one 10/100Mbps
and one 1Gbps interfaces.

Along with it I also included a fix to disable clocks for SiFive FU540-C000
on failure path of fu540_c000_clk_init().

Thank you,
Claudiu Beznea

Claudiu Beznea (7):
  net: macb: add userio bits as platform configuration
  net: macb: add capability to not set the clock rate
  net: macb: unprepare clocks in case of failure
  dt-bindings: add documentation for sama7g5 ethernet interface
  dt-bindings: add documentation for sama7g5 gigabit ethernet interface
  net: macb: add support for sama7g5 gem interface
  net: macb: add support for sama7g5 emac interface

 Documentation/devicetree/bindings/net/macb.txt |  2 +
 drivers/net/ethernet/cadence/macb.h            | 11 +++
 drivers/net/ethernet/cadence/macb_main.c       | 99 +++++++++++++++++++++-----
 3 files changed, 93 insertions(+), 19 deletions(-)

Comments

Andrew Lunn Dec. 5, 2020, 2:30 p.m. UTC | #1
On Fri, Dec 04, 2020 at 02:34:17PM +0200, Claudiu Beznea wrote:
> Unprepare clocks in case of any failure in fu540_c000_clk_init().

Hi Claudiu

Nice patchset. Simple to understand.
> 

> +err_disable_clocks:
> +	clk_disable_unprepare(*tx_clk);

> +	clk_disable_unprepare(*hclk);
> +	clk_disable_unprepare(*pclk);
> +	clk_disable_unprepare(*rx_clk);
> +	clk_disable_unprepare(*tsu_clk);

This looks correct, but it would be more symmetrical to add a

macb_clk_uninit()

function for the four main clocks. I'm surprised it does not already
exist.

	Andrew
Andrew Lunn Dec. 5, 2020, 2:30 p.m. UTC | #2
On Fri, Dec 04, 2020 at 02:34:15PM +0200, Claudiu Beznea wrote:
> This is necessary for SAMA7G5 as it uses different values for
> PHY interface and also introduces hdfctlen bit.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Andrew Lunn Dec. 5, 2020, 2:32 p.m. UTC | #3
On Fri, Dec 04, 2020 at 02:34:16PM +0200, Claudiu Beznea wrote:
> SAMA7G5's ethernet IPs TX clock could be provided by its generic clock or
> by the external clock provided by the PHY. The internal IP logic divides
> properly this clock depending on the link speed. The patch adds a new
> capability so that macb_set_tx_clock() to not be called for IPs having
> this capability (the clock rate, in case of generic clock, is set at the
> boot time via device tree and the driver only enables it).
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Andrew Lunn Dec. 5, 2020, 2:33 p.m. UTC | #4
On Fri, Dec 04, 2020 at 02:34:20PM +0200, Claudiu Beznea wrote:
> Add support for SAMA7G5 gigabit ethernet interface.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Andrew Lunn Dec. 5, 2020, 2:33 p.m. UTC | #5
On Fri, Dec 04, 2020 at 02:34:21PM +0200, Claudiu Beznea wrote:
> Add support for SAMA7G5 10/100Mbps interface.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Claudiu Beznea Dec. 7, 2020, 9:26 a.m. UTC | #6
Hi Andrew,

On 05.12.2020 16:30, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Dec 04, 2020 at 02:34:17PM +0200, Claudiu Beznea wrote:
>> Unprepare clocks in case of any failure in fu540_c000_clk_init().
> 
> Hi Claudiu
> 
> Nice patchset. Simple to understand.
>>
> 
>> +err_disable_clocks:
>> +     clk_disable_unprepare(*tx_clk);
> 
>> +     clk_disable_unprepare(*hclk);
>> +     clk_disable_unprepare(*pclk);
>> +     clk_disable_unprepare(*rx_clk);
>> +     clk_disable_unprepare(*tsu_clk);
> 
> This looks correct, but it would be more symmetrical to add a
> 
> macb_clk_uninit()
> 
> function for the four main clocks. I'm surprised it does not already
> exist.

I was in balance b/w added and not added it taking into account that the
disable unprepares are not taking care of all the clocks, in all the
places, in the same way. Anyway, I will add one function for the main
clocks, as you proposed, in the next version.

Thank you for your review,
Claudiu

> 
>         Andrew
>