Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
Hi, On Tue, Aug 27, 2013 at 01:27:48PM -0700, Julius Werner wrote: *Ping!* Are there still unanswered concerns left with this patch? I hope my prior mails could clear up why I think that the PMU register description in the device tree for a specific PHY is represents the hardware more accurately after my patch, and my analysis of the Exynos4 situation currently not being implemented (and therefore not needing to be addressed by this patch) was correct. Please let me know if you have further objections... and if not, could we agree to have this picked up somewhere? I need acks for DTS part... -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
Hi Felipe, On Tuesday 17 of September 2013 10:36:16 Felipe Balbi wrote: Hi, On Tue, Aug 27, 2013 at 01:27:48PM -0700, Julius Werner wrote: *Ping!* Are there still unanswered concerns left with this patch? I hope my prior mails could clear up why I think that the PMU register description in the device tree for a specific PHY is represents the hardware more accurately after my patch, and my analysis of the Exynos4 situation currently not being implemented (and therefore not needing to be addressed by this patch) was correct. Please let me know if you have further objections... and if not, could we agree to have this picked up somewhere? I need acks for DTS part... This patch breaks Exynos 4, so it's a NAK from me. Anyway, a new, completely redesigned PHY driver supporting most of the PHY features (as opposed to only the basic subset currently) developed by Kamil Debski should show up soon, so I don't think there is any reason to apply any patches to this old driver. Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Tue, Sep 17, 2013 at 05:53:31PM +0200, Tomasz Figa wrote: Hi Felipe, On Tuesday 17 of September 2013 10:36:16 Felipe Balbi wrote: Hi, On Tue, Aug 27, 2013 at 01:27:48PM -0700, Julius Werner wrote: *Ping!* Are there still unanswered concerns left with this patch? I hope my prior mails could clear up why I think that the PMU register description in the device tree for a specific PHY is represents the hardware more accurately after my patch, and my analysis of the Exynos4 situation currently not being implemented (and therefore not needing to be addressed by this patch) was correct. Please let me know if you have further objections... and if not, could we agree to have this picked up somewhere? I need acks for DTS part... This patch breaks Exynos 4, so it's a NAK from me. Anyway, a new, completely redesigned PHY driver supporting most of the PHY features (as opposed to only the basic subset currently) developed by Kamil Debski should show up soon, so I don't think there is any reason to apply any patches to this old driver. awesome, then I espect to see a patch deleting the old driver shortly :-) cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
*Ping!* Are there still unanswered concerns left with this patch? I hope my prior mails could clear up why I think that the PMU register description in the device tree for a specific PHY is represents the hardware more accurately after my patch, and my analysis of the Exynos4 situation currently not being implemented (and therefore not needing to be addressed by this patch) was correct. Please let me know if you have further objections... and if not, could we agree to have this picked up somewhere? Thanks, Julius -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Wed, Aug 07, 2013 at 06:06:05PM +0100, Julius Werner wrote: This breaks compatibility, both for an old kernel and a new dt and a new kernel with an old dt. Is anyone using these bindings? They only affect Samsung SoCs and have only been upstream for half a year, so I doubt it's heavily used. I'm not sure everyone will be happy with that line. Why are we describing fewer registers now? Are they described elsewhere? The dt should describe the device, not only the portion of it Linux wants to use right now. This only ever described a small section of the huge set of PMU registers anyway. Before it described up to three registers controlling different PHYs (using hardcoded offsets in the code to later find the right one)... with my patch every PHY's DT entry only describes the one register concerning itself, which makes more sense in my opinion. It will also prevent the register descriptions in different DT entries from overlapping. I'm not sure I understand. The old documentation referred to the USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers for a phy, and your new version only refers to (usb device) PHY_CONTROL. Regardless of multiple phys, you're suggesting that we describe less of each phy. That seems like taking away usable information. Unless I've misunderstood? Ideally, we'd describe the whole set of registers and linkages to phys, even if Linux doesn't ahppen to use that information right now. Thanks, Mark. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
I'm not sure I understand. The old documentation referred to the USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers for a phy, and your new version only refers to (usb device) PHY_CONTROL. Regardless of multiple phys, you're suggesting that we describe less of each phy. That seems like taking away usable information. Unless I've misunderstood? Well that's just the thing that's confusing right now, and which I am trying to fix: every PHY is either DEVICE or HOST and thus has only one PMU register. The current code describes the PMU register space for all PHYs on the system in the DT entry of every PHY and then calculates which register to use with hardcoded offsets. I think it makes much more sense if every PHY only describes its own register and doesn't need to do address arithmetic later on. As Vivek said there is one exception in an old Exynos4, but that is currently not implemented in the upstream kernel anyway, and if it ever will be it's still much easier to special case one weird chip than to have a super complicated and confusing mechanism for all of them. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
Hi Julius, On Thursday 08 of August 2013 11:06:54 Julius Werner wrote: I'm not sure I understand. The old documentation referred to the USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers for a phy, and your new version only refers to (usb device) PHY_CONTROL. Regardless of multiple phys, you're suggesting that we describe less of each phy. That seems like taking away usable information. Unless I've misunderstood? Well that's just the thing that's confusing right now, and which I am trying to fix: every PHY is either DEVICE or HOST and thus has only one PMU register. The current code describes the PMU register space for all PHYs on the system in the DT entry of every PHY and then calculates which register to use with hardcoded offsets. I think it makes much more sense if every PHY only describes its own register and doesn't need to do address arithmetic later on. As Vivek said there is one exception in an old Exynos4, Not that old yet. :) but that is currently not implemented in the upstream kernel anyway Sorry, I don't understand what is not implemented. Without your patch, the PHY driver handles both PMU registers of Exynos4. Best regards, Tomasz , and if it ever will be it's still much easier to special case one weird chip than to have a super complicated and confusing mechanism for all of them. -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
Sorry, I don't understand what is not implemented. Without your patch, the PHY driver handles both PMU registers of Exynos4. I don't have an Exynos4 to actually test this, so please let me know if I'm missing something here... but in order to hit the right HOST PHY register in the current upstream code, the Exynos4 code would need to have a hostphy_reg_offset of 4 somewhere in its samsung_usbphy_drvdata. In my latest checkout of Linus' tree (6c2580c) it does not (only Exynos5 sets that attribute), so it would default to 0 (thereby actually hitting the DEVICE register). If you want I can gladly provide another change on top of my patchset to fix that in the future... but it looks to me like it had been broken anyway for now. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Tue, Aug 06, 2013 at 07:00:17PM +0100, Julius Werner wrote:
This patch simplifies the way the phy-samsung-usb code finds the correct
power management register to enable PHY clock gating. Previously, the
code would calculate the register address from a device tree supplied
base address and add an offset based on the PHY type.
Since every PHY has its own device tree entry and needs only one
register, we can just encode the address itself in the device tree and
remove the diffentiation in the code. The bitmask needed to specify the
bit within that register stays in place, allowing support for platforms
like s3c64xx that use different bits within the same register.
This breaks compatibility, both for an old kernel and a new dt and a new
kernel with an old dt. Is anyone using these bindings?
Signed-off-by: Julius Werner jwer...@chromium.org
---
.../devicetree/bindings/usb/samsung-usbphy.txt | 26
+-
arch/arm/boot/dts/exynos5250.dtsi | 4 ++--
drivers/usb/phy/phy-samsung-usb.c | 18 ---
drivers/usb/phy/phy-samsung-usb.h | 23 +--
drivers/usb/phy/phy-samsung-usb2.c | 11 -
drivers/usb/phy/phy-samsung-usb3.c | 2 +-
6 files changed, 22 insertions(+), 62 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 33fd354..1cf9b68 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -34,14 +34,7 @@ Optional properties:
- The child node 'usbphy-sys' to the node 'usbphy' is for the system
controller
interface for usb-phy. It should provide the following information
required by
usb-phy controller to control phy.
- - reg : base physical address of PHY_CONTROL registers.
- The size of this register is the total sum of size of all PHY_CONTROL
- registers that the SoC has. For example, the size will be
- '0x4' in case we have only one PHY_CONTROL register (e.g.
- OTHERS register in S3C64XX or USB_PHY_CONTROL register in S5PV210)
- and, '0x8' in case we have two PHY_CONTROL registers (e.g.
- USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers in exynos4x).
- and so on.
+ - reg : address of PHY_CONTROL register for this PHY.
Example:
- Exynos4210
@@ -57,8 +50,8 @@ Example:
clock-names = xusbxti, otg;
usbphy-sys {
- /* USB device and host PHY_CONTROL registers */
- reg = 0x10020704 0x8;
+ /* USB device PHY_CONTROL register */
+ reg = 0x10020704 0x4;
};
};
Why are we describing fewer registers now? Are they described elsewhere?
The dt should describe the device, not only the portion of it Linux
wants to use right now.
Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
This patch simplifies the way the phy-samsung-usb code finds the correct
power management register to enable PHY clock gating. Previously, the
code would calculate the register address from a device tree supplied
base address and add an offset based on the PHY type.
Since every PHY has its own device tree entry and needs only one
register, we can just encode the address itself in the device tree and
remove the diffentiation in the code. The bitmask needed to specify the
bit within that register stays in place, allowing support for platforms
like s3c64xx that use different bits within the same register.
Signed-off-by: Julius Werner jwer...@chromium.org
---
.../devicetree/bindings/usb/samsung-usbphy.txt | 26 +-
arch/arm/boot/dts/exynos5250.dtsi | 4 ++--
drivers/usb/phy/phy-samsung-usb.c | 18 ---
drivers/usb/phy/phy-samsung-usb.h | 23 +--
drivers/usb/phy/phy-samsung-usb2.c | 11 -
drivers/usb/phy/phy-samsung-usb3.c | 2 +-
6 files changed, 22 insertions(+), 62 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
index 33fd354..1cf9b68 100644
--- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
+++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt
@@ -34,14 +34,7 @@ Optional properties:
- The child node 'usbphy-sys' to the node 'usbphy' is for the system controller
interface for usb-phy. It should provide the following information required
by
usb-phy controller to control phy.
- - reg : base physical address of PHY_CONTROL registers.
- The size of this register is the total sum of size of all PHY_CONTROL
- registers that the SoC has. For example, the size will be
- '0x4' in case we have only one PHY_CONTROL register (e.g.
- OTHERS register in S3C64XX or USB_PHY_CONTROL register in S5PV210)
- and, '0x8' in case we have two PHY_CONTROL registers (e.g.
- USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers in exynos4x).
- and so on.
+ - reg : address of PHY_CONTROL register for this PHY.
Example:
- Exynos4210
@@ -57,8 +50,8 @@ Example:
clock-names = xusbxti, otg;
usbphy-sys {
- /* USB device and host PHY_CONTROL registers */
- reg = 0x10020704 0x8;
+ /* USB device PHY_CONTROL register */
+ reg = 0x10020704 0x4;
};
};
@@ -90,14 +83,7 @@ Optional properties:
- The child node 'usbphy-sys' to the node 'usbphy' is for the system controller
interface for usb-phy. It should provide the following information required
by
usb-phy controller to control phy.
- - reg : base physical address of PHY_CONTROL registers.
- The size of this register is the total sum of size of all PHY_CONTROL
- registers that the SoC has. For example, the size will be
- '0x4' in case we have only one PHY_CONTROL register (e.g.
- OTHERS register in S3C64XX or USB_PHY_CONTROL register in S5PV210)
- and, '0x8' in case we have two PHY_CONTROL registers (e.g.
- USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL registers in exynos4x).
- and so on.
+ - reg : address of PHY_CONTROL register for this PHY.
Example:
usbphy@1210 {
@@ -111,7 +97,7 @@ Example:
clock-names = ext_xtal, usbdrd30;
usbphy-sys {
- /* USB device and host PHY_CONTROL registers */
- reg = 0x10040704 0x8;
+ /* USB DRD PHY_CONTROL register */
+ reg = 0x10040704 0x4;
};
};
diff --git a/arch/arm/boot/dts/exynos5250.dtsi
b/arch/arm/boot/dts/exynos5250.dtsi
index ef57277..5a754d7 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -473,7 +473,7 @@
ranges;
usbphy-sys {
- reg = 0x10040704 0x8;
+ reg = 0x10040704 0x4;
};
};
@@ -505,7 +505,7 @@
ranges;
usbphy-sys {
- reg = 0x10040704 0x8,
+ reg = 0x10040708 0x4,
0x10050230 0x4;
};
};
diff --git a/drivers/usb/phy/phy-samsung-usb.c
b/drivers/usb/phy/phy-samsung-usb.c
index ac025ca..51cad19 100644
--- a/drivers/usb/phy/phy-samsung-usb.c
+++ b/drivers/usb/phy/phy-samsung-usb.c
@@ -75,31 +75,21 @@ EXPORT_SYMBOL_GPL(samsung_usbphy_parse_dt);
*/
void samsung_usbphy_set_isolation_4210(struct samsung_usbphy *sphy, bool on)
{
- void __iomem *reg = NULL;
u32 reg_val;
- u32 en_mask = 0;
if (!sphy-pmuregs) {
dev_warn(sphy-dev, Can't set pmu
