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

[Sebastian Hesselbarth]

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.

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

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