Re: [linux-sunxi] Re: [PATCH v3 1/2] phy-sun4i-usb: Use of_match_node to get model specific config data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

On 27-11-15 09:53, Maxime Ripard wrote:
On Thu, Nov 26, 2015 at 01:11:32PM +0100, Hans de Goede wrote:
+enum sun4i_usb_phy_type {
+       sun4i_a10_phy,
+       sun8i_a33_phy,
+};
+
+struct sun4i_usb_phy_cfg {
+       int num_phys;
+       u32 disc_thresh;
+       enum sun4i_usb_phy_type type;
+       bool dedicated_clocks;
+};
+
  struct sun4i_usb_phy_data {
         void __iomem *base;
+       const struct sun4i_usb_phy_cfg *cfg;
         struct mutex mutex;
-       int num_phys;
-       u32 disc_thresh;
-       bool has_a33_phyctl;
         struct sun4i_usb_phy {
                 struct phy *phy;
                 void __iomem *pmu;
@@ -164,12 +174,15 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, u32 addr, u32 data,

         mutex_lock(&phy_data->mutex);

-       if (phy_data->has_a33_phyctl) {
+       switch (phy_data->cfg->type) {
+       case sun4i_a10_phy:
+               phyctl = phy_data->base + REG_PHYCTL_A10;

Any reason why this offset isn't incorporated into phy_data?

You mean in phy_data->cfg I assume, the difference needed for
the "sun4i_usb_phy_write" functionality are not just the phyctl
register offset...


+               break;
+       case sun8i_a33_phy:
                 phyctl = phy_data->base + REG_PHYCTL_A33;
                 /* A33 needs us to set phyctl to 0 explicitly */
                 writel(0, phyctl);

e.g. the A33 needs this extra write, and on the H3 we need to do
similar bitbanging, but slightly different, see:

https://github.com/allwinner-zh/linux-3.4-sunxi/blob/master/drivers/usb/host/sunxi_hci.c#L899

Notice how it uses different addr and write register addresses
their through the usb_phy_csr_add and usb_phy_csr_write helper
functions as well as directly poking offset 0x20.

Then it easy to support: one u8 for each register that changes, one
bool to tell if you need to clear the phyctl register or not, And you
don't have to duplicate the switch everywhere, and basically just
reimplement of_device_is_compatible without an actual compatible to
workaround the review ;)

You clearly have not looked at the actual code I've linked to, the
entire "algorithm" for sun4i_usb_phy_write is different on the H3.

Moreover, this has nothing to do with this patch, the code coding the
difference behavior for the a10 style phy and the a33 style phy is already
there before this patch-set, and this is not something Kishon asked
me to change.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux