Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)

[Edgar E. Iglesias]

Sorry for reposting,
My email client freaked out when posting the first reply...

----- Forwarded message from "Edgar E. Iglesias" <address@hidden> -----

Return-Path: <address@hidden>
Received: from localhost (rocksteady.se.axis.com. [195.60.68.156])
        by mx.google.com with ESMTPS id s9sm9668542lbc.12.2013.01.24.05.24.07
        (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128);
        Thu, 24 Jan 2013 05:24:07 -0800 (PST)
Date: Thu, 24 Jan 2013 14:24:06 +0100
From: "Edgar E. Iglesias" <address@hidden>
To: Grant Likely <address@hidden>
Cc: Paul Brook <address@hidden>,
        address@hidden,
        _Peter_Maydell_ <address@hidden>,
        "_ Edgar_E._Iglesias_" <address@hidden>,
        _Anthony_Liguori_ <address@hidden>,
        _Andreas_F=E4rber_ <address@hidden>, address@hidden
Subject: Re: [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging
        emulation
Message-ID: <address@hidden>
References: <address@hidden>
        <address@hidden>
        <address@hidden>
        <address@hidden>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <address@hidden>
User-Agent: Mutt/1.5.21 (2010-09-15)

On Thu, Jan 24, 2013 at 10:32:12AM +0000, Grant Likely wrote:
> On Wed, 23 Jan 2013 23:45:13 +0000, Paul Brook <address@hidden> wrote:
> > > +#ifdef USE_THIS_DEAD_CODE
> > > +void mdio_detach(struct qemu_mdio *bus, struct qemu_phy *phy, unsigned 
> > > int
> > > addr) +{
> > > +    bus->devs[addr & 0x1f] = NULL;
> > > +}
> > > +#endif
> > 
> > This is clearly wrong.
> 
> It's in both versions of the original code. I didn't add this. I
> included it when moving a code block because it appears to be there as a
> point of completeness if it ever should be needed.
> 
> Edgar, do you want to keep this block around?

If there are no users we can remove it

> 
> > It also worries me that there isn't a clean separation between the MDIO bus 
> > and the bitbang interface.  IMO the bitbang interface should be a separate 
> > device, and if we're wiring up bitbang interfaces then it really should be 
> > via 
> > standard GPIO pins (aka qemu_irq). 
> 
> Only the bitbang state machine is in the mdio layer. It says nothing
> about where those signals come from, gpio or otherwise. Not all cases
> will actually be GPIOs. For instance, the smc91c111 has dedicated pins
> for MDIO operations which are not GPIOs, even though the driver has to
> manage the bigbanging.
> 
> That said, I'm not opposed to changing the model if that is the design
> direction. However, I hope that the series won't be blocked on this
> point. This series moves and enhances existing code. A move to qemu_irq
> should be done as a follow-on patch.


Maybe we should do it like the i2c framework? It does very similar
things as mdio would need (with a nice split). It addresses Pauls comments
(I think) and also the split between slaves and the bus. It also makes it
possible to select PHY model from board code.

Best regards,
Edgar

----- End forwarded message -----