HI,

quoted

quoted

quoted

quoted

quoted

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.

Correct me if I am wrong here. If the same PHY is used for PCIe, SATA,
and USB, would you not need to indicate what speed the PHY should be
operated at - unless the underlying IP magically negotiate and
configured automatically? If so, what about PHY isn't so intelligent?
How should you suggest that we handle these?

quoted

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.

Testing purpose is only one use case. Another use case is to limit the
speed so that I can confirm the driver actually works with various
speed of the device and handle correctly.

quoted

quoted

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.

This PHY isn't as "intelligent" as other PHY's. What would you suggest
as I need an method to indicate to the underlying PHY that I want to
operate at an specified speed?

-Loc