Re: [PATCH 3/4] net: mvneta: mmc: get optional axi clk

[Jisheng Zhang]

Dear Sebastian,

On Wed, 20 Jan 2016 12:03:03 +0100 Sebastian Hesselbarth wrote:

> On 01/20/2016 10:42 AM, Jisheng Zhang wrote:
> > On Wed, 20 Jan 2016 10:31:18 +0100 Sebastian Hesselbarth  wrote:
> >  
> >> On January 20, 2016 9:15:22 AM Jisheng Zhang wrote:
> >>  
> >>> Some platforms may provide more than one clk for the mvneta IP, for
> >>> example Marvell BG4CT provides "core" clk for the mac core, and
> >>> "axi" clk for the AXI bus logic. Obviously this "axi" clk also need to
> >>> be enabled. This patch adds this optional "axi" clk support.  
> >>
> >> Jisheng,
> >>
> >> although I do not expect mvneta to appear on a non-AXI bus
> >> anytime soon, how about naming the clock "bus" instead?  
> >
> > Good question. IIRC, this IP expects AXI bus, but I'll check with HW people.  
> 
> Actually, I am quite sure the current IP requires AXI. But my comment
> was more about to make the binding a little bit more flexible to
> _future_ variants/SoCs we may stumble upon.

Got your points. PS: the clk is for AXI bus logic, so "bus" makes sense.

> 
> Naming the clock "bus" or "dma" will work for the current _and_ future
> IPs, while "axi" may not.

Indeed, will cook a v3

Thanks a lot for review,
Jisheng

> 
> Sebastian
> 
> >>
> >> If you know the clock is only required for bus master DMA but
> >> not for register access, "dma" would be an even better name.
> >>
> >> Sebastian
> >>
> >>  
> >>> Signed-off-by: Jisheng Zhang <jszhang@xxxxxxxxxxx>
> >>> ---
> >>>   drivers/net/ethernet/marvell/mvneta.c | 8 ++++++++
> >>>   1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/drivers/net/ethernet/marvell/mvneta.c
> >>> b/drivers/net/ethernet/marvell/mvneta.c
> >>> index aca0a73..6bb709a 100644
> >>> --- a/drivers/net/ethernet/marvell/mvneta.c
> >>> +++ b/drivers/net/ethernet/marvell/mvneta.c
> >>> @@ -373,6 +373,8 @@ struct mvneta_port {
> >>>
> >>>   	/* Core clock */
> >>>   	struct clk *clk;
> >>> +	/* AXI clock */
> >>> +	struct clk *clk_axi;
> >>>   	u8 mcast_count[256];
> >>>   	u16 tx_ring_size;
> >>>   	u16 rx_ring_size;
> >>> @@ -3615,6 +3617,10 @@ static int mvneta_probe(struct platform_device *pdev)
> >>>
> >>>   	clk_prepare_enable(pp->clk);
> >>>
> >>> +	pp->clk_axi = devm_clk_get(&pdev->dev, "axi");
> >>> +	if (!IS_ERR(pp->clk_axi))
> >>> +		clk_prepare_enable(pp->clk_axi);
> >>> +
> >>>   	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >>>   	pp->base = devm_ioremap_resource(&pdev->dev, res);
> >>>   	if (IS_ERR(pp->base)) {
> >>> @@ -3727,6 +3733,7 @@ err_free_ports:
> >>>   	free_percpu(pp->ports);
> >>>   err_clk:
> >>>   	clk_disable_unprepare(pp->clk);
> >>> +	clk_disable_unprepare(pp->clk_axi);
> >>>   err_put_phy_node:
> >>>   	of_node_put(phy_node);
> >>>   err_free_irq:
> >>> @@ -3744,6 +3751,7 @@ static int mvneta_remove(struct platform_device *pdev)
> >>>
> >>>   	unregister_netdev(dev);
> >>>   	clk_disable_unprepare(pp->clk);
> >>> +	clk_disable_unprepare(pp->clk_axi);
> >>>   	free_percpu(pp->ports);
> >>>   	free_percpu(pp->stats);
> >>>   	irq_dispose_mapping(dev->irq);
> >>> --
> >>> 2.7.0.rc3
> >>>  
> >>
> >>  
> >