Thread (2 messages) 2 messages, 2 authors, 2013-01-09

[PATCH v8 1/2] usb: phy: samsung: Introducing usb phy driver for hsotg

From: Praveen Paneri <hidden>
Date: 2013-01-09 05:58:07
Also in: linux-devicetree, linux-samsung-soc

Hi Tomasz,

Sorry! I I missed this comment of yours. Is it okay if we keep
pmu_isolation as it is (as it does not seem much out of the line). We
have already gone through a lot of rework and there has been no
fruitful result :(
Also after Viveks work, this is only milited to non DT SoCs.

Thanks,
Praveen

On Wed, Nov 28, 2012 at 6:32 PM, Tomasz Figa [off-list ref] wrote:
Hi Praveen,

On Friday 23 of November 2012 09:56:37 Praveen Paneri wrote:
quoted
quoted
quoted
+static void samsung_usbphy_enable(struct samsung_usbphy *sphy)
+{
+     void __iomem *regs = sphy->regs;
+     u32 phypwr;
+     u32 phyclk;
+     u32 rstcon;
+
+     /* set clock frequency for PLL */
+     phyclk = sphy->ref_clk_freq;
+     phypwr = readl(regs + SAMSUNG_PHYPWR);
+     rstcon = readl(regs + SAMSUNG_RSTCON);
+
+     switch (sphy->cpu_type) {
+     case TYPE_S3C64XX:
+             phyclk &= ~PHYCLK_COMMON_ON_N;
+             phypwr &= ~PHYPWR_NORMAL_MASK;
+             rstcon |= RSTCON_SWRST;
+             break;
+     case TYPE_EXYNOS4210:
+             phypwr &= ~PHYPWR_NORMAL_MASK_PHY0;
+             rstcon |= RSTCON_SWRST;
+     default:
+             break;
+     }
+
+     writel(phyclk, regs + SAMSUNG_PHYCLK);
+     /* set to normal of PHY0 */
I don't understand this comment.
Will change it to " Configure PHY0 for normal operation"
That should be more clear, I suppose.
Yes, much better.
quoted
quoted
quoted
+ */
+
+#ifndef __SAMSUNG_USBPHY_PLATFORM_H
+#define __SAMSUNG_USBPHY_PLATFORM_H
+
+/**
+ * samsung_usbphy_data - Platform data for USB PHY driver.
+ * @pmu_isolation: Function to control usb phy isolation in PMU.
+ */
+struct samsung_usbphy_data {
+     void (*pmu_isolation)(int on);
I believe this should be named in a generic way. This is called PMU
isolation on Exynos SoCs, but on S3C64xx it's USB PHY mask.
Yes! I am aware of it. The fact that this ( MASK or ISOLATION) has
always been part of the PMU, pmu_isolation seems quite generic that
way. Though you can suggest a better name.
What do you think about set_isolation(int on) or power_isolation(int on)?
Sorry! I don't understand the point here.Why to invest so much of time
and evergy on things we want to remove soon.
I am a bit reluctant to change this now, after sending 9 versions of
the same code.
Lets concentrate on more important things like AUXDATA removal or
adding support for all DT enabled machines.
Best regards,
--
Tomasz Figa
Samsung Poland R&D Center
SW Solution Development, Linux Platform

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help