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
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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, 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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Thu, Aug 8, 2013 at 2:56 PM, Mark Rutland wrote: > 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? Just giving some pointers here: As also mentioned in the documentation for samsung-usbphy, SoCs prior to exynos4x had only one PMU register handling power to the PHYs (USB 2.0 phy to be specific). Exynos4x SoCs also had USB 2.0 PHY only but device and host PHYs' power was being handled by two registers namely - USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL. Exynos5x series of SoCs now have USB 2.0 type PHY (both device and host PHY are power-handled by only one register) and USB 3.0 type PHY (having a separate PMU register to handle power to PHY); so in a total of two registers but both handling entirely separate PHYs. So, samsung-usb2 phy driver should be interacting with only one PMU register (with an exception for exynos4x) and furthermore samsung-usb3phy driver interact with its separate PMU register. Sylwester, Please correct me if i am wrong somewhere. > > 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-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On Thu, Aug 8, 2013 at 2:56 PM, Mark Rutland mark.rutl...@arm.com wrote: 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? Just giving some pointers here: As also mentioned in the documentation for samsung-usbphy, SoCs prior to exynos4x had only one PMU register handling power to the PHYs (USB 2.0 phy to be specific). Exynos4x SoCs also had USB 2.0 PHY only but device and host PHYs' power was being handled by two registers namely - USBDEVICE_PHY_CONTROL and USBHOST_PHY_CONTROL. Exynos5x series of SoCs now have USB 2.0 type PHY (both device and host PHY are power-handled by only one register) and USB 3.0 type PHY (having a separate PMU register to handle power to PHY); so in a total of two registers but both handling entirely separate PHYs. So, samsung-usb2 phy driver should be interacting with only one PMU register (with an exception for exynos4x) and furthermore samsung-usb3phy driver interact with its separate PMU register. Sylwester, Please correct me if i am wrong somewhere. 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-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On 08/07/2013 07:06 PM, 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. It probably wouldn't be of much concern, but I can't tell for sure. >> 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. The PMU registers are already scattered across various drivers, like Power Domain, various PHYs (USB/HSIC/MIPI CSI-2/DSI, ADC, ...), Reset and more. And it happens currently there is no central driver for the Power Management Unit, that would, for example expose the regmap interface that the interested drivers could use. The downside of this would be that each, e.g. USB PHY driver would need to know SoC specific offsets to its registers in the PMU. Before device tree started to be used those PMU registers were controlled by respective drivers, mostly through platform_data callbacks. As they could be considered parts of given device. 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. Yes, the patch looks sensible. Since currently each USB PHY has its own device tree node it looks wrong to have the reg property in the usbphy-sys subnodes defining register region for all possible PHYs. And we indeed and up with multiple reg properties pointing to same register region. However the biggest drawback is breaking backwards compatibility. I'm not sure if those changes are worth it, especially that all those USB PHY drivers are supposed to be rewritten to use the generic PHY API. Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
> 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. > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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
> ---
> .../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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
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. 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. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3 v5] usb: phy-samsung-usb: Simplify PMU register handling
On 08/07/2013 07:06 PM, 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. It probably wouldn't be of much concern, but I can't tell for sure. 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. The PMU registers are already scattered across various drivers, like Power Domain, various PHYs (USB/HSIC/MIPI CSI-2/DSI, ADC, ...), Reset and more. And it happens currently there is no central driver for the Power Management Unit, that would, for example expose the regmap interface that the interested drivers could use. The downside of this would be that each, e.g. USB PHY driver would need to know SoC specific offsets to its registers in the PMU. Before device tree started to be used those PMU registers were controlled by respective drivers, mostly through platform_data callbacks. As they could be considered parts of given device. 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. Yes, the patch looks sensible. Since currently each USB PHY has its own device tree node it looks wrong to have the reg property in the usbphy-sys subnodes defining register region for all possible PHYs. And we indeed and up with multiple reg properties pointing to same register region. However the biggest drawback is breaking backwards compatibility. I'm not sure if those changes are worth it, especially that all those USB PHY drivers are supposed to be rewritten to use the generic PHY API. Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[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
---
.../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
[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
