Hi, On Thu, Feb 27, 2014 at 01:09:57PM -0800, Loc Ho wrote: > >> >> This patch adds function set_speed to the generic PHY framework > >> >> operation > >> >> structure. This function can be called to instruct the PHY underlying > >> >> layer > >> >> at specified lane to configure for specified speed in hertz. > >> > > >> > why ? looks like clk_set_rate() is your friend here. Can you be more > >> > descriptive of the use case ? When will this be used ? > >> > > >> > >> The phy_set_speed is used to configure the operation speed of the PHY > >> at run-time. The clock interface in general is used to configure the > >> clock input to the IP. I don't believe they are the same thing. Maybe > >> it will be clear in my response to your second email > > > > The problem with this is that you end up adding SATA-specific details to > > something which is supposed to be generic. > > Setting the operation speed is pretty generic from an interface point > of view. An generic multi-purpose PHY can support multiple speed. If
no it's not. Specially when you consider that your "speed" argument can be just about anything and depending on the underlying bus, it *will* be treated differently. Note that e.g. in OMAP devices the exact *same* PHY IP is used for PCIe, SATA and USB... now, let's assume for the sake of argument that we were to implement ->set_speed() in that environment, how do you plan to do that ? a 6MHz arguments isn't valid from USB stand point and could mean different things in PCIe or SATA. > the upper layer wish to operate at an specified speed (say for testing > purpose and etc), it can be allowed. anything for testing purposes, should be limited to test scenarios. > > After negoatiation, don't you > > get any interrupt from your PHY indicating that link speed negotiation > > is done ? Or is that IRQ only on AHCI IP ? > > There is NO interrupt from the PHY. The IRQ is assoicated with the > AHCI IP. With SATA host flow, it starts off with an COMRESET to start > the link negotiation. At that point, it will poll for completion. > > Any other concerns? hey, calm down... just trying to prevent us from applying something which isn't truly generic and I don't think "->set_speed" is generic enough. The semantics of the "speed" argument won't be valid for all use cases. I can already see people using that to pass USB_SPEED_{LOW,FULL,HIGH,SUPER}, instead of actual speed numbers. We wil end up with a mess to handle from a PHY driver, specially in cases such as OMAP where, as mentioned above, the same IP is used for SATA, PCIe and USB3. -- balbi
signature.asc
Description: Digital signature