Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)
Am 25.01.2013 19:00, schrieb Paul Brook: 1. It's not point-to-point, has an arbitrary nr of connection points. QoM currently only does asymmetric 1-1 connections between objects. However I don't think this is a fatal problem. We can still retain an asymmetric API (effectively equivalent to male and female physical connectors), adding virtual wire objects where they don't match up. It should be possible to implement this as a backward compatible extension to qemu_irq[1]. In most cases the additional wire should not be needed. This sounds pretty much like Anthony's Pin series, no? He had shown some of us a preview git branch but Peter didn't like the amount of boilerplate code to use it IIRC. Haven't heard any update since... Andreas For simple output-input (i.e. all existing code) we just need to ignore Z states. Preferably before they get to the input device. For simple bidirectional point-point lines (which should include bitbang-i2c and bitbang-mdio) the bitbang object controls the value when subject to a Z output. For arbitrary pin connections they all connect to a set of ports on a virtual wire device. It takes care of arbitrating line state and sending notifications to the connected devices. There are a couple of technical issues: Fristly qemu_irq is currently stateless[2]. Giving it state is fine in principle, but means a lot of load/save code needs fixing. In pactice we can probably avoid this, but there are some nice benefits from keeping state in qemu_irq. Secondly, the [parent of the] qemu_irq object needs to be able to signal value changes to the object on the other side of the link. Currently QoM allows a property to be linked to an object, but provides no way for the object to identify/communicate with the property/device linked to it. Paul [1] I've no particular attachment to the name qemu_irq. But I really don't want to have to make anything other than purely mechanical changes to all its users. [2] More precicely it has no state that changes over its lifetime. -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)
On Fri, Jan 25, 2013 at 06:00:36PM +, Paul Brook wrote: To be able to create generic GPIO devices or other devices that have GPIO like pins (e.g MDIO), and hook those up to external buses through common frameworks, we need agreement on how to model tristate pins. A tristate pin model, or at least agreement on how to model these with multiple qemu_irqs. hmm, feels like we've opened a can of worms... Probably. I'm not going to insist you use/implement generic GPIO for MDIO, but I still think separation between the PHY register interface and the bitbang code is good (i.e. same as bitbang_i2c). Sounds, good. Anyway, how would such a qemu_tristate_pin be modelled? [point 1 moved later so my answers read in a sensible order] 2. Every connection point provides an output/value and an output_enable. I think we'd be better providing a single state i.e. an output of 0, 1 or Z. Possibly additional Z0/Z1 states to represent high-impedance with pull-up/down resistors. Makes sense, the output output-enable are parts of the user of the tristate_pin.. 3. There is a mean for reading the pin value, which is computed based on all connection points outputs and output_enables (can be cached). For MDIO being able to read the value is sufficient. However in general we don't want to have to poll it. We want to be told when it changes. Agreed 4. The pin value can be invalid (multiple drivers or no drivers), 0 or 1. I can't think of any cases where this is important. In most cases it's undefined, in the rest it causes physical damage. I thought it might be useful for debugging, not sure how valueable it would be though... 1. It's not point-to-point, has an arbitrary nr of connection points. QoM currently only does asymmetric 1-1 connections between objects. However I don't think this is a fatal problem. We can still retain an asymmetric API (effectively equivalent to male and female physical connectors), adding virtual wire objects where they don't match up. It should be possible to implement this as a backward compatible extension to qemu_irq[1]. In most cases the additional wire should not be needed. For simple output-input (i.e. all existing code) we just need to ignore Z states. Preferably before they get to the input device. For simple bidirectional point-point lines (which should include bitbang-i2c and bitbang-mdio) the bitbang object controls the value when subject to a Z output. For arbitrary pin connections they all connect to a set of ports on a virtual wire device. It takes care of arbitrating line state and sending notifications to the connected devices. There are a couple of technical issues: Fristly qemu_irq is currently stateless[2]. Giving it state is fine in principle, but means a lot of load/save code needs fixing. In pactice we can probably avoid this, but there are some nice benefits from keeping state in qemu_irq. Secondly, the [parent of the] qemu_irq object needs to be able to signal value changes to the object on the other side of the link. Currently QoM allows a property to be linked to an object, but provides no way for the object to identify/communicate with the property/device linked to it. Thanks for clarifying Cheers, Edgar Paul [1] I've no particular attachment to the name qemu_irq. But I really don't want to have to make anything other than purely mechanical changes to all its users. [2] More precicely it has no state that changes over its lifetime.
Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)
To be able to create generic GPIO devices or other devices that have GPIO like pins (e.g MDIO), and hook those up to external buses through common frameworks, we need agreement on how to model tristate pins. A tristate pin model, or at least agreement on how to model these with multiple qemu_irqs. hmm, feels like we've opened a can of worms... Probably. I'm not going to insist you use/implement generic GPIO for MDIO, but I still think separation between the PHY register interface and the bitbang code is good (i.e. same as bitbang_i2c). Anyway, how would such a qemu_tristate_pin be modelled? [point 1 moved later so my answers read in a sensible order] 2. Every connection point provides an output/value and an output_enable. I think we'd be better providing a single state i.e. an output of 0, 1 or Z. Possibly additional Z0/Z1 states to represent high-impedance with pull-up/down resistors. 3. There is a mean for reading the pin value, which is computed based on all connection points outputs and output_enables (can be cached). For MDIO being able to read the value is sufficient. However in general we don't want to have to poll it. We want to be told when it changes. 4. The pin value can be invalid (multiple drivers or no drivers), 0 or 1. I can't think of any cases where this is important. In most cases it's undefined, in the rest it causes physical damage. 1. It's not point-to-point, has an arbitrary nr of connection points. QoM currently only does asymmetric 1-1 connections between objects. However I don't think this is a fatal problem. We can still retain an asymmetric API (effectively equivalent to male and female physical connectors), adding virtual wire objects where they don't match up. It should be possible to implement this as a backward compatible extension to qemu_irq[1]. In most cases the additional wire should not be needed. For simple output-input (i.e. all existing code) we just need to ignore Z states. Preferably before they get to the input device. For simple bidirectional point-point lines (which should include bitbang-i2c and bitbang-mdio) the bitbang object controls the value when subject to a Z output. For arbitrary pin connections they all connect to a set of ports on a virtual wire device. It takes care of arbitrating line state and sending notifications to the connected devices. There are a couple of technical issues: Fristly qemu_irq is currently stateless[2]. Giving it state is fine in principle, but means a lot of load/save code needs fixing. In pactice we can probably avoid this, but there are some nice benefits from keeping state in qemu_irq. Secondly, the [parent of the] qemu_irq object needs to be able to signal value changes to the object on the other side of the link. Currently QoM allows a property to be linked to an object, but provides no way for the object to identify/communicate with the property/device linked to it. Paul [1] I've no particular attachment to the name qemu_irq. But I really don't want to have to make anything other than purely mechanical changes to all its users. [2] More precicely it has no state that changes over its lifetime.
Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)
Sorry for reposting,
My email client freaked out when posting the first reply...
- Forwarded message from Edgar E. Iglesias edgar.igles...@gmail.com
-
Return-Path: edgar.igles...@gmail.com
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 edgar.igles...@gmail.com
To: Grant Likely grant.lik...@secretlab.ca
Cc: Paul Brook p...@codesourcery.com,
=?ISO-8859-1?Q?qemu-devel@nongnu.org,
_Peter_Maydell_ peter.ma?==?ISO-8859-1?Q?yd...@linaro.org,
_ Edgar_E._Iglesias_ edgar.?==?ISO-8859-1?Q?igles...@gmail.com,
_Anthony_Liguori_ aliguori?==?ISO-8859-1?Q?@us.ibm.com,
_Andreas_F=E4rber_ afaer...@suse.de, ?=@edde.se.axis.com
Subject: Re: [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging
emulation
Message-ID: 20130124132406.gc10...@edde.se.axis.com
References: 1358957730-17897-1-git-send-email-grant.lik...@secretlab.ca
1358957730-17897-3-git-send-email-grant.lik...@secretlab.ca
201301232345.13669.p...@codesourcery.com
20130124103212.BE2473E249D@localhost
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 20130124103212.BE2473E249D@localhost
User-Agent: Mutt/1.5.21 (2010-09-15)
On Thu, Jan 24, 2013 at 10:32:12AM +, Grant Likely wrote:
On Wed, 23 Jan 2013 23:45:13 +, Paul Brook p...@codesourcery.com 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 -
Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)
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. There's no such thing as a dedicated pin managed by software. That's exactly what a GPIO pin is. It may be that particular pins are usually used for a particular purpose, but I don't think that is sufficient reason to create a whole new API. The way to solve that is to give the pins appropriate names. Don't be distracted by the fact that the smc91c111 is two devices (MAC and PHY) on the same chip. 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. Yes. Though on closer inspection the bitbang I2C module introduces bitbang_i2c_set, which I'd preter to avoid. This isn't quite as easy as it should be because we don't have a nice solution for tristate pins (currently modelled as a cross-wired output and input pair). Paul
Re: [Qemu-devel] [PATCH V2 2/6] hw/mdio: Generalize etraxfs MDIO bitbanging emulation (fwd)
On Thu, Jan 24, 2013 at 02:21:09PM +, Paul Brook wrote: 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. There's no such thing as a dedicated pin managed by software. That's exactly what a GPIO pin is. It may be that particular pins are usually used for a particular purpose, but I don't think that is sufficient reason to create a whole new API. The way to solve that is to give the pins appropriate names. Don't be distracted by the fact that the smc91c111 is two devices (MAC and PHY) on the same chip. 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. Yes. Though on closer inspection the bitbang I2C module introduces bitbang_i2c_set, which I'd preter to avoid. This isn't quite as easy as it should be because we don't have a nice solution for tristate pins (currently modelled as a cross-wired output and input pair). I see what you mean.. To be able to create generic GPIO devices or other devices that have GPIO like pins (e.g MDIO), and hook those up to external buses through common frameworks, we need agreement on how to model tristate pins. A tristate pin model, or at least agreement on how to model these with multiple qemu_irqs. hmm, feels like we've opened a can of worms... Anyway, how would such a qemu_tristate_pin be modelled? 1. It's not point-to-point, has an arbitrary nr of connection points. 2. Every connection point provides an output/value and an output_enable. 3. There is a mean for reading the pin value, which is computed based on all connection points outputs and output_enables (can be cached). 4. The pin value can be invalid (multiple drivers or no drivers), 0 or 1. Am I missing something? Cheers
