Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 3/6/2025 5:25 AM, Dmitry Baryshkov wrote: > On Wed, Mar 05, 2025 at 06:20:45PM +0800, Xiangxu Yin wrote: >> >> >> On 12/20/2024 8:01 AM, Dmitry Baryshkov wrote: >>> On Wed, Dec 18, 2024 at 08:55:54PM +0800, Xiangxu Yin wrote: On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: > On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: >> >> >> On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: >>> On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: >>> >>> >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: > > Extended DP support for QCS615 USB or DP phy. Differentiated > between > USBC and DP PHY using the match table’s type, dynamically > generating > different types of cfg and layout attributes during > initialization based > on this type. Static variables are stored in cfg, while parsed > values > are organized into the layout structure. We didn't have an understanding / conclusion whether qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY or two PHYs being placed next to each other. Could you please start your commit message by explaining it? Or even better, make that a part of the cover letter for a new series touching just the USBC PHY driver. DP changes don't have anything in common with the PHY changes, so you can split the series into two. >>> Before implement DP extension, we have discussed with abhinav and >>> krishna about whether use combo, usbc or separate phy. >> >> What is "DP extension"? >> I'm sorry confusion casued by my description. It's means extend DP implemnt for USBC phy driver. >>> >>> We identified that DP and USB share some common controls for >>> phy_mode and orientation. >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the >>> lanes - USB or DP, >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the >>> orientation. >>> It would be more efficient for a single driver to manage these >>> controls. >> >> The question is about the hardware, not about the driver. >> >>> Additionally, this PHY does not support Alt Mode, and the two >>> control registers are located in separate address spaces. >>> Therefore, even though the orientation for DP on this platform is >>> always normal and connected to the video output board, >>> we still decided to base it on the USBC extension. >> >> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I >> thought that usbc-or-dp platforms support that, but they don't >> support DP+USB pin configuration. Note, the question is broader than >> just QCS615, it covers the PHY type itself. >> >> Also, is TCSR configuration read/write or read-only? Are we supposed >> to >> set the register from OS or are we supposed to read it and thus >> detemine >> the PHY mode? > > Any updates on these two topics? > Still confirming detail info with HW & design team. I’ll update the information that has been confirmed so far. This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane work for DP, other 2 lane work for USB. TCSR phy mode is read/write reg and we can read for determine phy mode. >>> >>> Ok, thanks for the explanation. From my point of view: >>> >>> - Implement the DP PHY to be a part of the same driver. Each device >>> supported by the usbc driver should get both PHYs. >>> >>> - Make sure not to break the ABI: #phy-cells = <0> should still work and >>> return USB PHY, keeping backwards compatibility. Newer devices or >>> upgraded DT for old devices should return USB PHY for <... 0> and DP >>> PHY for <... 1>. >>> >> Yes, currently we have implemented like your description, >> Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... >> 0>. > > Please note the backwards compatibility clause. > For the USB node, we kept the same implementation as the original function interface, and the devicetree node definition also remains unchanged. In subsequent patches, I will follow Krzyszto
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Fri, Mar 21, 2025 at 06:17:39PM +0800, Xiangxu Yin wrote: > > > On 3/6/2025 5:25 AM, Dmitry Baryshkov wrote: > > On Wed, Mar 05, 2025 at 06:20:45PM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/20/2024 8:01 AM, Dmitry Baryshkov wrote: > >>> On Wed, Dec 18, 2024 at 08:55:54PM +0800, Xiangxu Yin wrote: > > > On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: > > On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > >>> On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: > > > On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > >>> > >>> > >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > wrote: > > > > Extended DP support for QCS615 USB or DP phy. Differentiated > > between > > USBC and DP PHY using the match table’s type, dynamically > > generating > > different types of cfg and layout attributes during > > initialization based > > on this type. Static variables are stored in cfg, while parsed > > values > > are organized into the layout structure. > > We didn't have an understanding / conclusion whether > qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device > / PHY > or two PHYs being placed next to each other. Could you please > start > your commit message by explaining it? Or even better, make that > a part > of the cover letter for a new series touching just the USBC PHY > driver. DP changes don't have anything in common with the PHY > changes, > so you can split the series into two. > > >>> Before implement DP extension, we have discussed with abhinav and > >>> krishna about whether use combo, usbc or separate phy. > >> > >> What is "DP extension"? > >> > I'm sorry confusion casued by my description. It's means extend DP > implemnt for USBC phy driver. > >>> > >>> We identified that DP and USB share some common controls for > >>> phy_mode and orientation. > >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the > >>> lanes - USB or DP, > >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the > >>> orientation. > >>> It would be more efficient for a single driver to manage these > >>> controls. > >> > >> The question is about the hardware, not about the driver. > >> > >>> Additionally, this PHY does not support Alt Mode, and the two > >>> control registers are located in separate address spaces. > >>> Therefore, even though the orientation for DP on this platform is > >>> always normal and connected to the video output board, > >>> we still decided to base it on the USBC extension. > >> > >> Could you please clarify, do usb3-or-dp PHYs support > >> DP-over-USB-C? I > >> thought that usbc-or-dp platforms support that, but they don't > >> support DP+USB pin configuration. Note, the question is broader > >> than > >> just QCS615, it covers the PHY type itself. > >> > >> Also, is TCSR configuration read/write or read-only? Are we > >> supposed to > >> set the register from OS or are we supposed to read it and thus > >> detemine > >> the PHY mode? > > > > Any updates on these two topics? > > > Still confirming detail info with HW & design team. > I’ll update the information that has been confirmed so far. > This phy support DP-over-USB-C,but it's not support alt-mode which 2 > lane work for DP, other 2 lane work for USB. > TCSR phy mode is read/write reg and we can read for determine phy > mode. > >>> > >>> Ok, thanks for the explanation. From my point of view: > >>> > >>> - Implement the DP PHY to be a part of the same driver. Each device > >>> supported by the usbc driver should get both PHYs. > >>> > >>> - Make sure not to break the ABI: #phy-cells = <0> should still work > >>> and > >>> return USB PHY, keeping backwards compatibility. Newer devices or > >>> upgraded DT for old devices should return USB PHY for <... 0> and DP > >>> PHY for <... 1>. > >>> > >> Yes, currently we have implemented like your description, > >> Each deivce shoud get
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Wed, Mar 05, 2025 at 06:20:45PM +0800, Xiangxu Yin wrote: > > > On 12/20/2024 8:01 AM, Dmitry Baryshkov wrote: > > On Wed, Dec 18, 2024 at 08:55:54PM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: > >>> On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: > > > On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > > On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > >>> On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > > > > > > On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > >> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > >> wrote: > >>> > >>> Extended DP support for QCS615 USB or DP phy. Differentiated > >>> between > >>> USBC and DP PHY using the match table’s type, dynamically > >>> generating > >>> different types of cfg and layout attributes during > >>> initialization based > >>> on this type. Static variables are stored in cfg, while parsed > >>> values > >>> are organized into the layout structure. > >> > >> We didn't have an understanding / conclusion whether > >> qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / > >> PHY > >> or two PHYs being placed next to each other. Could you please start > >> your commit message by explaining it? Or even better, make that a > >> part > >> of the cover letter for a new series touching just the USBC PHY > >> driver. DP changes don't have anything in common with the PHY > >> changes, > >> so you can split the series into two. > >> > > Before implement DP extension, we have discussed with abhinav and > > krishna about whether use combo, usbc or separate phy. > > What is "DP extension"? > > >> I'm sorry confusion casued by my description. It's means extend DP > >> implemnt for USBC phy driver. > > > > We identified that DP and USB share some common controls for > > phy_mode and orientation. > > Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the > > lanes - USB or DP, > > while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the > > orientation. > > It would be more efficient for a single driver to manage these > > controls. > > The question is about the hardware, not about the driver. > > > Additionally, this PHY does not support Alt Mode, and the two > > control registers are located in separate address spaces. > > Therefore, even though the orientation for DP on this platform is > > always normal and connected to the video output board, > > we still decided to base it on the USBC extension. > > Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I > thought that usbc-or-dp platforms support that, but they don't > support DP+USB pin configuration. Note, the question is broader than > just QCS615, it covers the PHY type itself. > > Also, is TCSR configuration read/write or read-only? Are we supposed > to > set the register from OS or are we supposed to read it and thus > detemine > the PHY mode? > >>> > >>> Any updates on these two topics? > >>> > >> Still confirming detail info with HW & design team. > >> I’ll update the information that has been confirmed so far. > >> This phy support DP-over-USB-C,but it's not support alt-mode which 2 > >> lane work for DP, other 2 lane work for USB. > >> TCSR phy mode is read/write reg and we can read for determine phy mode. > > > > Ok, thanks for the explanation. From my point of view: > > > > - Implement the DP PHY to be a part of the same driver. Each device > > supported by the usbc driver should get both PHYs. > > > > - Make sure not to break the ABI: #phy-cells = <0> should still work and > > return USB PHY, keeping backwards compatibility. Newer devices or > > upgraded DT for old devices should return USB PHY for <... 0> and DP > > PHY for <... 1>. > > > Yes, currently we have implemented like your description, > Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... > 0>. > >>> > >>> Please note the backwards compatibility clause. > >>> > >> For the USB node, we kept the same implementation as the original function > >> interface, and the devicetree node definition also remains unchanged. > >> In subsequent patches, I will follow Krzysztof’s suggestion to use a > >> separate DT-binding t
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 12/20/2024 8:01 AM, Dmitry Baryshkov wrote: > On Wed, Dec 18, 2024 at 08:55:54PM +0800, Xiangxu Yin wrote: >> >> >> On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: >>> On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: >> >> >> On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: >>> On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > > > On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: >> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin >> wrote: >>> >>> Extended DP support for QCS615 USB or DP phy. Differentiated between >>> USBC and DP PHY using the match table’s type, dynamically generating >>> different types of cfg and layout attributes during initialization >>> based >>> on this type. Static variables are stored in cfg, while parsed >>> values >>> are organized into the layout structure. >> >> We didn't have an understanding / conclusion whether >> qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY >> or two PHYs being placed next to each other. Could you please start >> your commit message by explaining it? Or even better, make that a >> part >> of the cover letter for a new series touching just the USBC PHY >> driver. DP changes don't have anything in common with the PHY >> changes, >> so you can split the series into two. >> > Before implement DP extension, we have discussed with abhinav and > krishna about whether use combo, usbc or separate phy. What is "DP extension"? >> I'm sorry confusion casued by my description. It's means extend DP >> implemnt for USBC phy driver. > > We identified that DP and USB share some common controls for phy_mode > and orientation. > Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the > lanes - USB or DP, > while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the > orientation. > It would be more efficient for a single driver to manage these > controls. The question is about the hardware, not about the driver. > Additionally, this PHY does not support Alt Mode, and the two control > registers are located in separate address spaces. > Therefore, even though the orientation for DP on this platform is > always normal and connected to the video output board, > we still decided to base it on the USBC extension. Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I thought that usbc-or-dp platforms support that, but they don't support DP+USB pin configuration. Note, the question is broader than just QCS615, it covers the PHY type itself. Also, is TCSR configuration read/write or read-only? Are we supposed to set the register from OS or are we supposed to read it and thus detemine the PHY mode? >>> >>> Any updates on these two topics? >>> >> Still confirming detail info with HW & design team. >> I’ll update the information that has been confirmed so far. >> This phy support DP-over-USB-C,but it's not support alt-mode which 2 >> lane work for DP, other 2 lane work for USB. >> TCSR phy mode is read/write reg and we can read for determine phy mode. > > Ok, thanks for the explanation. From my point of view: > > - Implement the DP PHY to be a part of the same driver. Each device > supported by the usbc driver should get both PHYs. > > - Make sure not to break the ABI: #phy-cells = <0> should still work and > return USB PHY, keeping backwards compatibility. Newer devices or > upgraded DT for old devices should return USB PHY for <... 0> and DP > PHY for <... 1>. > Yes, currently we have implemented like your description, Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... 0>. >>> >>> Please note the backwards compatibility clause. >>> >> For the USB node, we kept the same implementation as the original function >> interface, and the devicetree node definition also remains unchanged. >> In subsequent patches, I will follow Krzysztof’s suggestion to use a >> separate DT-binding to describe the DP PHY configuration, >> without making changes to the USB devicetree and DT-binding implementation. > - I'm not shure how to handle the USB and DP coexistence, especially in > your case of the USB-or-DP PHY. > For coexistence process: When we start implement DP part, usb driver
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Wed, Dec 18, 2024 at 08:55:54PM +0800, Xiangxu Yin wrote: > > > On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: > > On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > >>> On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: > > > On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > >>> > >>> > >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > wrote: > > > > Extended DP support for QCS615 USB or DP phy. Differentiated between > > USBC and DP PHY using the match table’s type, dynamically generating > > different types of cfg and layout attributes during initialization > > based > > on this type. Static variables are stored in cfg, while parsed > > values > > are organized into the layout structure. > > We didn't have an understanding / conclusion whether > qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY > or two PHYs being placed next to each other. Could you please start > your commit message by explaining it? Or even better, make that a > part > of the cover letter for a new series touching just the USBC PHY > driver. DP changes don't have anything in common with the PHY > changes, > so you can split the series into two. > > >>> Before implement DP extension, we have discussed with abhinav and > >>> krishna about whether use combo, usbc or separate phy. > >> > >> What is "DP extension"? > >> > I'm sorry confusion casued by my description. It's means extend DP > implemnt for USBC phy driver. > >>> > >>> We identified that DP and USB share some common controls for phy_mode > >>> and orientation. > >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the > >>> lanes - USB or DP, > >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the > >>> orientation. > >>> It would be more efficient for a single driver to manage these > >>> controls. > >> > >> The question is about the hardware, not about the driver. > >> > >>> Additionally, this PHY does not support Alt Mode, and the two control > >>> registers are located in separate address spaces. > >>> Therefore, even though the orientation for DP on this platform is > >>> always normal and connected to the video output board, > >>> we still decided to base it on the USBC extension. > >> > >> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I > >> thought that usbc-or-dp platforms support that, but they don't > >> support DP+USB pin configuration. Note, the question is broader than > >> just QCS615, it covers the PHY type itself. > >> > >> Also, is TCSR configuration read/write or read-only? Are we supposed to > >> set the register from OS or are we supposed to read it and thus > >> detemine > >> the PHY mode? > > > > Any updates on these two topics? > > > Still confirming detail info with HW & design team. > I’ll update the information that has been confirmed so far. > This phy support DP-over-USB-C,but it's not support alt-mode which 2 > lane work for DP, other 2 lane work for USB. > TCSR phy mode is read/write reg and we can read for determine phy mode. > >>> > >>> Ok, thanks for the explanation. From my point of view: > >>> > >>> - Implement the DP PHY to be a part of the same driver. Each device > >>> supported by the usbc driver should get both PHYs. > >>> > >>> - Make sure not to break the ABI: #phy-cells = <0> should still work and > >>> return USB PHY, keeping backwards compatibility. Newer devices or > >>> upgraded DT for old devices should return USB PHY for <... 0> and DP > >>> PHY for <... 1>. > >>> > >> Yes, currently we have implemented like your description, > >> Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... > >> 0>. > > > > Please note the backwards compatibility clause. > > > For the USB node, we kept the same implementation as the original function > interface, and the devicetree node definition also remains unchanged. > In subsequent patches, I will follow Krzysztof’s suggestion to use a separate > DT-binding to describe the DP PHY configuration, > without making changes to the USB devicetree and DT-binding implementation. > >>> - I'm not shure how to handle the USB and DP coexistence, especially in > >>> your case of the USB-or-DP PHY. > >>> > >> For coexistence process: > >> > >> When we start implement DP part, usb driver team said only need config > >> TCSR phy mode and orie
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Wed, Dec 18, 2024 at 08:55:54PM +0800, Xiangxu Yin wrote: > > > On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: > > On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > >>> On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: > > > On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > >>> > >>> > >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > wrote: > > > > Extended DP support for QCS615 USB or DP phy. Differentiated between > > USBC and DP PHY using the match table’s type, dynamically generating > > different types of cfg and layout attributes during initialization > > based > > on this type. Static variables are stored in cfg, while parsed > > values > > are organized into the layout structure. > > We didn't have an understanding / conclusion whether > qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY > or two PHYs being placed next to each other. Could you please start > your commit message by explaining it? Or even better, make that a > part > of the cover letter for a new series touching just the USBC PHY > driver. DP changes don't have anything in common with the PHY > changes, > so you can split the series into two. > > >>> Before implement DP extension, we have discussed with abhinav and > >>> krishna about whether use combo, usbc or separate phy. > >> > >> What is "DP extension"? > >> > I'm sorry confusion casued by my description. It's means extend DP > implemnt for USBC phy driver. > >>> > >>> We identified that DP and USB share some common controls for phy_mode > >>> and orientation. > >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the > >>> lanes - USB or DP, > >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the > >>> orientation. > >>> It would be more efficient for a single driver to manage these > >>> controls. > >> > >> The question is about the hardware, not about the driver. > >> > >>> Additionally, this PHY does not support Alt Mode, and the two control > >>> registers are located in separate address spaces. > >>> Therefore, even though the orientation for DP on this platform is > >>> always normal and connected to the video output board, > >>> we still decided to base it on the USBC extension. > >> > >> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I > >> thought that usbc-or-dp platforms support that, but they don't > >> support DP+USB pin configuration. Note, the question is broader than > >> just QCS615, it covers the PHY type itself. > >> > >> Also, is TCSR configuration read/write or read-only? Are we supposed to > >> set the register from OS or are we supposed to read it and thus > >> detemine > >> the PHY mode? > > > > Any updates on these two topics? > > > Still confirming detail info with HW & design team. > I’ll update the information that has been confirmed so far. > This phy support DP-over-USB-C,but it's not support alt-mode which 2 > lane work for DP, other 2 lane work for USB. > TCSR phy mode is read/write reg and we can read for determine phy mode. > >>> > >>> Ok, thanks for the explanation. From my point of view: > >>> > >>> - Implement the DP PHY to be a part of the same driver. Each device > >>> supported by the usbc driver should get both PHYs. > >>> > >>> - Make sure not to break the ABI: #phy-cells = <0> should still work and > >>> return USB PHY, keeping backwards compatibility. Newer devices or > >>> upgraded DT for old devices should return USB PHY for <... 0> and DP > >>> PHY for <... 1>. > >>> > >> Yes, currently we have implemented like your description, > >> Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... > >> 0>. > > > > Please note the backwards compatibility clause. > > > For the USB node, we kept the same implementation as the original function > interface, and the devicetree node definition also remains unchanged. > In subsequent patches, I will follow Krzysztof’s suggestion to use a separate > DT-binding to describe the DP PHY configuration, > without making changes to the USB devicetree and DT-binding implementation. NO! See below. But basically NAK for implementing a separate bindings and separate DP PHY driver. Also you can't just leave old platforms. All of them have the same hardware, so all these platforms can be updated simultaneously. > >>> - I'm not shure how to han
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 12/12/2024 3:15 AM, Dmitry Baryshkov wrote: > On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: >> >> >> On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: >>> On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: >>> >>> >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: > > Extended DP support for QCS615 USB or DP phy. Differentiated between > USBC and DP PHY using the match table’s type, dynamically generating > different types of cfg and layout attributes during initialization > based > on this type. Static variables are stored in cfg, while parsed values > are organized into the layout structure. We didn't have an understanding / conclusion whether qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY or two PHYs being placed next to each other. Could you please start your commit message by explaining it? Or even better, make that a part of the cover letter for a new series touching just the USBC PHY driver. DP changes don't have anything in common with the PHY changes, so you can split the series into two. >>> Before implement DP extension, we have discussed with abhinav and >>> krishna about whether use combo, usbc or separate phy. >> >> What is "DP extension"? >> I'm sorry confusion casued by my description. It's means extend DP implemnt for USBC phy driver. >>> >>> We identified that DP and USB share some common controls for phy_mode >>> and orientation. >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes >>> - USB or DP, >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the >>> orientation. >>> It would be more efficient for a single driver to manage these >>> controls. >> >> The question is about the hardware, not about the driver. >> >>> Additionally, this PHY does not support Alt Mode, and the two control >>> registers are located in separate address spaces. >>> Therefore, even though the orientation for DP on this platform is >>> always normal and connected to the video output board, >>> we still decided to base it on the USBC extension. >> >> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I >> thought that usbc-or-dp platforms support that, but they don't >> support DP+USB pin configuration. Note, the question is broader than >> just QCS615, it covers the PHY type itself. >> >> Also, is TCSR configuration read/write or read-only? Are we supposed to >> set the register from OS or are we supposed to read it and thus detemine >> the PHY mode? > > Any updates on these two topics? > Still confirming detail info with HW & design team. I’ll update the information that has been confirmed so far. This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane work for DP, other 2 lane work for USB. TCSR phy mode is read/write reg and we can read for determine phy mode. >>> >>> Ok, thanks for the explanation. From my point of view: >>> >>> - Implement the DP PHY to be a part of the same driver. Each device >>> supported by the usbc driver should get both PHYs. >>> >>> - Make sure not to break the ABI: #phy-cells = <0> should still work and >>> return USB PHY, keeping backwards compatibility. Newer devices or >>> upgraded DT for old devices should return USB PHY for <... 0> and DP >>> PHY for <... 1>. >>> >> Yes, currently we have implemented like your description, >> Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... 0>. > > Please note the backwards compatibility clause. > For the USB node, we kept the same implementation as the original function interface, and the devicetree node definition also remains unchanged. In subsequent patches, I will follow Krzysztof’s suggestion to use a separate DT-binding to describe the DP PHY configuration, without making changes to the USB devicetree and DT-binding implementation. >>> - I'm not shure how to handle the USB and DP coexistence, especially in >>> your case of the USB-or-DP PHY. >>> >> For coexistence process: >> >> When we start implement DP part, usb driver team said only need config TCSR >> phy mode and orientation during switch in USB-C port. >> Based on your previous comments avout SW_PWRDN, I'm confirming with the USB >> team whether SW_REST/SWPWRDN/START_CTRL registers might affect DP. > > Thanks! > >> Anyway, even though the original SoC design supports DP or USB over Type-C， >> but on QCS615 ADP AIR platform, th
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Wed, Dec 11, 2024 at 08:50:02PM +0800, Xiangxu Yin wrote: > > > On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > > On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: > >> > >> > >> On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > >>> On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > > > > > > On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > >> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > >> wrote: > >>> > >>> Extended DP support for QCS615 USB or DP phy. Differentiated between > >>> USBC and DP PHY using the match table’s type, dynamically generating > >>> different types of cfg and layout attributes during initialization > >>> based > >>> on this type. Static variables are stored in cfg, while parsed values > >>> are organized into the layout structure. > >> > >> We didn't have an understanding / conclusion whether > >> qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY > >> or two PHYs being placed next to each other. Could you please start > >> your commit message by explaining it? Or even better, make that a part > >> of the cover letter for a new series touching just the USBC PHY > >> driver. DP changes don't have anything in common with the PHY changes, > >> so you can split the series into two. > >> > > Before implement DP extension, we have discussed with abhinav and > > krishna about whether use combo, usbc or separate phy. > > What is "DP extension"? > > >> I'm sorry confusion casued by my description. It's means extend DP > >> implemnt for USBC phy driver. > > > > We identified that DP and USB share some common controls for phy_mode > > and orientation. > > Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes > > - USB or DP, > > while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the > > orientation. > > It would be more efficient for a single driver to manage these > > controls. > > The question is about the hardware, not about the driver. > > > Additionally, this PHY does not support Alt Mode, and the two control > > registers are located in separate address spaces. > > Therefore, even though the orientation for DP on this platform is > > always normal and connected to the video output board, > > we still decided to base it on the USBC extension. > > Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I > thought that usbc-or-dp platforms support that, but they don't > support DP+USB pin configuration. Note, the question is broader than > just QCS615, it covers the PHY type itself. > > Also, is TCSR configuration read/write or read-only? Are we supposed to > set the register from OS or are we supposed to read it and thus detemine > the PHY mode? > >>> > >>> Any updates on these two topics? > >>> > >> Still confirming detail info with HW & design team. > >> I’ll update the information that has been confirmed so far. > >> This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane > >> work for DP, other 2 lane work for USB. > >> TCSR phy mode is read/write reg and we can read for determine phy mode. > > > > Ok, thanks for the explanation. From my point of view: > > > > - Implement the DP PHY to be a part of the same driver. Each device > > supported by the usbc driver should get both PHYs. > > > > - Make sure not to break the ABI: #phy-cells = <0> should still work and > > return USB PHY, keeping backwards compatibility. Newer devices or > > upgraded DT for old devices should return USB PHY for <... 0> and DP > > PHY for <... 1>. > > > Yes, currently we have implemented like your description, > Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... 0>. Please note the backwards compatibility clause. > > - I'm not shure how to handle the USB and DP coexistence, especially in > > your case of the USB-or-DP PHY. > > > For coexistence process: > > When we start implement DP part, usb driver team said only need config TCSR > phy mode and orientation during switch in USB-C port. > Based on your previous comments avout SW_PWRDN, I'm confirming with the USB > team whether SW_REST/SWPWRDN/START_CTRL registers might affect DP. Thanks! > Anyway, even though the original SoC design supports DP or USB over Type-C， > but on QCS615 ADP AIR platform, there are only four USB-A port which works > with 'qcs615-qmp-usb3-phy' driver, and no USB-C port. > DP port is mappped from usb pin to the video out sub-board. > so we are unable to verify the switching case between DP and USB devices > under USB-C. That's also fine. We will get to that point once MSM8998 / SDM660 get USB-C support (the only current blocker is the support for the TYPEC block of the PMI8998). > How
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 12/11/2024 5:46 PM, Dmitry Baryshkov wrote: > On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: >> >> >> On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: >>> On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > > > On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: >> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin >> wrote: >>> >>> Extended DP support for QCS615 USB or DP phy. Differentiated between >>> USBC and DP PHY using the match table’s type, dynamically generating >>> different types of cfg and layout attributes during initialization based >>> on this type. Static variables are stored in cfg, while parsed values >>> are organized into the layout structure. >> >> We didn't have an understanding / conclusion whether >> qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY >> or two PHYs being placed next to each other. Could you please start >> your commit message by explaining it? Or even better, make that a part >> of the cover letter for a new series touching just the USBC PHY >> driver. DP changes don't have anything in common with the PHY changes, >> so you can split the series into two. >> > Before implement DP extension, we have discussed with abhinav and krishna > about whether use combo, usbc or separate phy. What is "DP extension"? >> I'm sorry confusion casued by my description. It's means extend DP implemnt >> for USBC phy driver. > > We identified that DP and USB share some common controls for phy_mode and > orientation. > Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - > USB or DP, > while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation. > It would be more efficient for a single driver to manage these controls. The question is about the hardware, not about the driver. > Additionally, this PHY does not support Alt Mode, and the two control > registers are located in separate address spaces. > Therefore, even though the orientation for DP on this platform is always > normal and connected to the video output board, > we still decided to base it on the USBC extension. Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I thought that usbc-or-dp platforms support that, but they don't support DP+USB pin configuration. Note, the question is broader than just QCS615, it covers the PHY type itself. Also, is TCSR configuration read/write or read-only? Are we supposed to set the register from OS or are we supposed to read it and thus detemine the PHY mode? >>> >>> Any updates on these two topics? >>> >> Still confirming detail info with HW & design team. >> I’ll update the information that has been confirmed so far. >> This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane >> work for DP, other 2 lane work for USB. >> TCSR phy mode is read/write reg and we can read for determine phy mode. > > Ok, thanks for the explanation. From my point of view: > > - Implement the DP PHY to be a part of the same driver. Each device > supported by the usbc driver should get both PHYs. > > - Make sure not to break the ABI: #phy-cells = <0> should still work and > return USB PHY, keeping backwards compatibility. Newer devices or > upgraded DT for old devices should return USB PHY for <... 0> and DP > PHY for <... 1>. > Yes, currently we have implemented like your description, Each deivce shoud get both PHYs, DP PHY for <... 1> and USB PHY for <... 0>. > - I'm not shure how to handle the USB and DP coexistence, especially in > your case of the USB-or-DP PHY. > For coexistence process: When we start implement DP part, usb driver team said only need config TCSR phy mode and orientation during switch in USB-C port. Based on your previous comments avout SW_PWRDN, I'm confirming with the USB team whether SW_REST/SWPWRDN/START_CTRL registers might affect DP. Anyway, even though the original SoC design supports DP or USB over Type-C， but on QCS615 ADP AIR platform, there are only four USB-A port which works with 'qcs615-qmp-usb3-phy' driver, and no USB-C port. DP port is mappped from usb pin to the video out sub-board. so we are unable to verify the switching case between DP and USB devices under USB-C. However, I'm also confirming whether anything other will affect USB and DP each other.
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Wed, Dec 11, 2024 at 08:46:16AM +0800, Xiangxu Yin wrote: > > > On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > >>> > >>> > >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > wrote: > > > > Extended DP support for QCS615 USB or DP phy. Differentiated between > > USBC and DP PHY using the match table’s type, dynamically generating > > different types of cfg and layout attributes during initialization based > > on this type. Static variables are stored in cfg, while parsed values > > are organized into the layout structure. > > We didn't have an understanding / conclusion whether > qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY > or two PHYs being placed next to each other. Could you please start > your commit message by explaining it? Or even better, make that a part > of the cover letter for a new series touching just the USBC PHY > driver. DP changes don't have anything in common with the PHY changes, > so you can split the series into two. > > >>> Before implement DP extension, we have discussed with abhinav and krishna > >>> about whether use combo, usbc or separate phy. > >> > >> What is "DP extension"? > >> > I'm sorry confusion casued by my description. It's means extend DP implemnt > for USBC phy driver. > >>> > >>> We identified that DP and USB share some common controls for phy_mode and > >>> orientation. > >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - > >>> USB or DP, > >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation. > >>> It would be more efficient for a single driver to manage these controls. > >> > >> The question is about the hardware, not about the driver. > >> > >>> Additionally, this PHY does not support Alt Mode, and the two control > >>> registers are located in separate address spaces. > >>> Therefore, even though the orientation for DP on this platform is always > >>> normal and connected to the video output board, > >>> we still decided to base it on the USBC extension. > >> > >> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I > >> thought that usbc-or-dp platforms support that, but they don't > >> support DP+USB pin configuration. Note, the question is broader than > >> just QCS615, it covers the PHY type itself. > >> > >> Also, is TCSR configuration read/write or read-only? Are we supposed to > >> set the register from OS or are we supposed to read it and thus detemine > >> the PHY mode? > > > > Any updates on these two topics? > > > Still confirming detail info with HW & design team. > I’ll update the information that has been confirmed so far. > This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane > work for DP, other 2 lane work for USB. > TCSR phy mode is read/write reg and we can read for determine phy mode. Ok, thanks for the explanation. From my point of view: - Implement the DP PHY to be a part of the same driver. Each device supported by the usbc driver should get both PHYs. - Make sure not to break the ABI: #phy-cells = <0> should still work and return USB PHY, keeping backwards compatibility. Newer devices or upgraded DT for old devices should return USB PHY for <... 0> and DP PHY for <... 1>. - I'm not shure how to handle the USB and DP coexistence, especially in your case of the USB-or-DP PHY. -- With best wishes Dmitry
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 12/10/2024 11:09 PM, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: >> On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: >>> >>> >>> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote: > > Extended DP support for QCS615 USB or DP phy. Differentiated between > USBC and DP PHY using the match table’s type, dynamically generating > different types of cfg and layout attributes during initialization based > on this type. Static variables are stored in cfg, while parsed values > are organized into the layout structure. We didn't have an understanding / conclusion whether qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY or two PHYs being placed next to each other. Could you please start your commit message by explaining it? Or even better, make that a part of the cover letter for a new series touching just the USBC PHY driver. DP changes don't have anything in common with the PHY changes, so you can split the series into two. >>> Before implement DP extension, we have discussed with abhinav and krishna >>> about whether use combo, usbc or separate phy. >> >> What is "DP extension"? >> I'm sorry confusion casued by my description. It's means extend DP implemnt for USBC phy driver. >>> >>> We identified that DP and USB share some common controls for phy_mode and >>> orientation. >>> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - >>> USB or DP, >>> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation. >>> It would be more efficient for a single driver to manage these controls. >> >> The question is about the hardware, not about the driver. >> >>> Additionally, this PHY does not support Alt Mode, and the two control >>> registers are located in separate address spaces. >>> Therefore, even though the orientation for DP on this platform is always >>> normal and connected to the video output board, >>> we still decided to base it on the USBC extension. >> >> Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I >> thought that usbc-or-dp platforms support that, but they don't >> support DP+USB pin configuration. Note, the question is broader than >> just QCS615, it covers the PHY type itself. >> >> Also, is TCSR configuration read/write or read-only? Are we supposed to >> set the register from OS or are we supposed to read it and thus detemine >> the PHY mode? > > Any updates on these two topics? > Still confirming detail info with HW & design team. I’ll update the information that has been confirmed so far. This phy support DP-over-USB-C,but it's not support alt-mode which 2 lane work for DP, other 2 lane work for USB. TCSR phy mode is read/write reg and we can read for determine phy mode.
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Thu, Dec 05, 2024 at 08:31:24PM +0200, Dmitry Baryshkov wrote: > On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote: > > > > > > On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote: > > > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin > > > wrote: > > >> > > >> Extended DP support for QCS615 USB or DP phy. Differentiated between > > >> USBC and DP PHY using the match table’s type, dynamically generating > > >> different types of cfg and layout attributes during initialization based > > >> on this type. Static variables are stored in cfg, while parsed values > > >> are organized into the layout structure. > > > > > > We didn't have an understanding / conclusion whether > > > qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY > > > or two PHYs being placed next to each other. Could you please start > > > your commit message by explaining it? Or even better, make that a part > > > of the cover letter for a new series touching just the USBC PHY > > > driver. DP changes don't have anything in common with the PHY changes, > > > so you can split the series into two. > > > > > Before implement DP extension, we have discussed with abhinav and krishna > > about whether use combo, usbc or separate phy. > > What is "DP extension"? > > > > > We identified that DP and USB share some common controls for phy_mode and > > orientation. > > Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - > > USB or DP, > > while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation. > > It would be more efficient for a single driver to manage these controls. > > The question is about the hardware, not about the driver. > > > Additionally, this PHY does not support Alt Mode, and the two control > > registers are located in separate address spaces. > > Therefore, even though the orientation for DP on this platform is always > > normal and connected to the video output board, > > we still decided to base it on the USBC extension. > > Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I > thought that usbc-or-dp platforms support that, but they don't > support DP+USB pin configuration. Note, the question is broader than > just QCS615, it covers the PHY type itself. > > Also, is TCSR configuration read/write or read-only? Are we supposed to > set the register from OS or are we supposed to read it and thus detemine > the PHY mode? Any updates on these two topics? -- With best wishes Dmitry
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote:
> On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote:
>>
>> Extended DP support for QCS615 USB or DP phy. Differentiated between
>> USBC and DP PHY using the match table’s type, dynamically generating
>> different types of cfg and layout attributes during initialization based
>> on this type. Static variables are stored in cfg, while parsed values
>> are organized into the layout structure.
>
> We didn't have an understanding / conclusion whether
> qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY
> or two PHYs being placed next to each other. Could you please start
> your commit message by explaining it? Or even better, make that a part
> of the cover letter for a new series touching just the USBC PHY
> driver. DP changes don't have anything in common with the PHY changes,
> so you can split the series into two.
>
Before implement DP extension, we have discussed with abhinav and krishna about
whether use combo, usbc or separate phy.
We identified that DP and USB share some common controls for phy_mode and
orientation.
Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - USB or
DP,
while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation.
It would be more efficient for a single driver to manage these controls.
Additionally, this PHY does not support Alt Mode, and the two control registers
are located in separate address spaces.
Therefore, even though the orientation for DP on this platform is always normal
and connected to the video output board,
we still decided to base it on the USBC extension.
>>
>> Signed-off-by: Xiangxu Yin
>> ---
>> drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h |1 +
>> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1453
>>
>
> Too many changes for a single patch. Please split into logical chunks.
>
Ok.
Once we have clarified the overall direction,
we can then discuss whether to split on current list or create a new list for
the split.
>> 2 files changed, 1254 insertions(+), 200 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
>> b/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
>> index
>> 0ebd405bcaf0cac8215550bfc9b226f30cc43a59..59885616405f878885d0837838a0bac1899fb69f
>> 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
>> @@ -25,6 +25,7 @@
>> #define QSERDES_DP_PHY_AUX_CFG70x03c
>> #define QSERDES_DP_PHY_AUX_CFG80x040
>> #define QSERDES_DP_PHY_AUX_CFG90x044
>> +#define QSERDES_DP_PHY_VCO_DIV 0x068
>>
>> /* QSERDES COM_BIAS_EN_CLKBUFLR_EN bits */
>> # define QSERDES_V3_COM_BIAS_EN0x0001
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> index
>> cf12a6f12134dc77ff032f967b2efa43e3de4b21..7fece9d7dc959ed5a7c62077d8552324c3734859
>> 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> @@ -22,13 +22,20 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>>
>> #include "phy-qcom-qmp-common.h"
>>
>> #include "phy-qcom-qmp.h"
>> #include "phy-qcom-qmp-pcs-misc-v3.h"
>>
>> +#include "phy-qcom-qmp-dp-phy.h"
>> +#include "phy-qcom-qmp-dp-phy-v3.h"
>> +
>> #define PHY_INIT_COMPLETE_TIMEOUT 1
>> +#define SW_PORTSELECT_VAL BIT(0)
>> +#define SW_PORTSELECT_MUX BIT(1)
>>
>> /* set of registers with offsets different per-PHY */
>> enum qphy_reg_layout {
>> @@ -284,7 +291,26 @@ static const struct qmp_phy_init_tbl
>> qcm2290_usb3_pcs_tbl[] = {
>> QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_LVL, 0x88),
>> };
>>
>> -struct qmp_usbc_offsets {
>> +enum qmp_phy_usbc_type {
>> + QMP_PHY_USBC_INVALID,
>
> How can a type be invalid?
>
I thought that platformdata must specify a type, so I set the default value to
‘invalid’.
I will remove this in a future patch.
>> + QMP_PHY_USBC_USB,
>> + QMP_PHY_USBC_DP,
>> +};
>> +
>> +/* list of regulators */
>> +struct qmp_regulator_data {
>> + const char *name;
>> + unsigned int enable_load;
>> +};
>> +
>> +struct dev_cfg {
>> + int type;
>> + const void *cfg;
>> +};
>> +
>> +struct qmp_usbc;
>> +
>> +struct qmp_usbc_usb_offsets {
>> u16 serdes;
>> u16 pcs;
>> u16 pcs_misc;
>> @@ -295,9 +321,9 @@ struct qmp_usbc_offsets {
>> u16 rx2;
>> };
>>
>> -/* struct qmp_phy_cfg - per-PHY initialization config */
>> -struct qmp_phy_cfg {
>> - const struct qmp_usbc_offsets *offsets;
>> +/* struct qmp_phy_usb_cfg - per-usb PHY initialization config */
>
> what is "per-usb PHY"?
>
Each usb phy in which defined in platform data.
Shall I remove this annotation?
>> +struct qmp_phy_usb_cfg {
>> + const s
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Thu, Dec 05, 2024 at 09:26:47PM +0800, Xiangxu Yin wrote:
>
>
> On 11/29/2024 10:33 PM, Dmitry Baryshkov wrote:
> > On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote:
> >>
> >> Extended DP support for QCS615 USB or DP phy. Differentiated between
> >> USBC and DP PHY using the match table’s type, dynamically generating
> >> different types of cfg and layout attributes during initialization based
> >> on this type. Static variables are stored in cfg, while parsed values
> >> are organized into the layout structure.
> >
> > We didn't have an understanding / conclusion whether
> > qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY
> > or two PHYs being placed next to each other. Could you please start
> > your commit message by explaining it? Or even better, make that a part
> > of the cover letter for a new series touching just the USBC PHY
> > driver. DP changes don't have anything in common with the PHY changes,
> > so you can split the series into two.
> >
> Before implement DP extension, we have discussed with abhinav and krishna
> about whether use combo, usbc or separate phy.
What is "DP extension"?
>
> We identified that DP and USB share some common controls for phy_mode and
> orientation.
> Specifically, 'TCSR_USB3_0_DP_PHYMODE' controls who must use the lanes - USB
> or DP,
> while PERIPH_SS_USB0_USB3PHY_PCS_MISC_TYPEC_CTRL controls the orientation.
> It would be more efficient for a single driver to manage these controls.
The question is about the hardware, not about the driver.
> Additionally, this PHY does not support Alt Mode, and the two control
> registers are located in separate address spaces.
> Therefore, even though the orientation for DP on this platform is always
> normal and connected to the video output board,
> we still decided to base it on the USBC extension.
Could you please clarify, do usb3-or-dp PHYs support DP-over-USB-C? I
thought that usbc-or-dp platforms support that, but they don't
support DP+USB pin configuration. Note, the question is broader than
just QCS615, it covers the PHY type itself.
Also, is TCSR configuration read/write or read-only? Are we supposed to
set the register from OS or are we supposed to read it and thus detemine
the PHY mode?
Anyway, judging on my understanding the platform configuration should
contain both USB and DP bits with the driver registering a single PHY
which supports switching between USB and DP (TCSR register is R/W) or
a single PHY which provides USBC or DP functionality depending on the
TCSR register contents (if it is R/O)
Andwhat is "USBC extension", BTW?
> >>
> >> Signed-off-by: Xiangxu Yin
> >> ---
> >> drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h |1 +
> >> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1453
> >>
> >
> > Too many changes for a single patch. Please split into logical chunks.
> >
> Ok.
> Once we have clarified the overall direction,
> we can then discuss whether to split on current list or create a new list for
> the split.
> >> 2 files changed, 1254 insertions(+), 200 deletions(-)
> >>
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> >> b/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> >> index
> >> 0ebd405bcaf0cac8215550bfc9b226f30cc43a59..59885616405f878885d0837838a0bac1899fb69f
> >> 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> >> @@ -25,6 +25,7 @@
> >> #define QSERDES_DP_PHY_AUX_CFG70x03c
> >> #define QSERDES_DP_PHY_AUX_CFG80x040
> >> #define QSERDES_DP_PHY_AUX_CFG90x044
> >> +#define QSERDES_DP_PHY_VCO_DIV 0x068
> >>
> >> /* QSERDES COM_BIAS_EN_CLKBUFLR_EN bits */
> >> # define QSERDES_V3_COM_BIAS_EN0x0001
> >> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> >> b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> >> index
> >> cf12a6f12134dc77ff032f967b2efa43e3de4b21..7fece9d7dc959ed5a7c62077d8552324c3734859
> >> 100644
> >> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> >> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> >> @@ -22,13 +22,20 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> +#include
> >>
> >> #include "phy-qcom-qmp-common.h"
> >>
> >> #include "phy-qcom-qmp.h"
> >> #include "phy-qcom-qmp-pcs-misc-v3.h"
> >>
> >> +#include "phy-qcom-qmp-dp-phy.h"
> >> +#include "phy-qcom-qmp-dp-phy-v3.h"
> >> +
> >> #define PHY_INIT_COMPLETE_TIMEOUT 1
> >> +#define SW_PORTSELECT_VAL BIT(0)
> >> +#define SW_PORTSELECT_MUX BIT(1)
> >>
> >> /* set of registers with offsets different per-PHY */
> >> enum qphy_reg_layout {
> >> @@ -284,7 +291,26 @@ static const struct qmp_phy_init_tbl
> >> qcm2290_usb3_pcs_tbl[] = {
> >> QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_LVL, 0x88),
> >> };
> >>
> >> -struct qmp_u
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 11/29/2024 4:18 PM, Krzysztof Kozlowski wrote:
> On 29/11/2024 08:57, Xiangxu Yin wrote:
>> Extended DP support for QCS615 USB or DP phy. Differentiated between
>> USBC and DP PHY using the match table’s type, dynamically generating
>> different types of cfg and layout attributes during initialization based
>> on this type. Static variables are stored in cfg, while parsed values
>> are organized into the layout structure.
>>
>> Signed-off-by: Xiangxu Yin
>> ---
>> drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h |1 +
>> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1453
>>
>> 2 files changed, 1254 insertions(+), 200 deletions(-)
>
>
>
> ...
>
>> +/* program default setting first */
>> +writel(0x2A, tx + QSERDES_V3_TX_TX_DRV_LVL);
>> +writel(0x20, tx + QSERDES_V3_TX_TX_EMP_POST1_LVL);
>> +writel(0x2A, tx2 + QSERDES_V3_TX_TX_DRV_LVL);
>> +writel(0x20, tx2 + QSERDES_V3_TX_TX_EMP_POST1_LVL);
>> +
>> +writel(voltage_swing_cfg, tx + QSERDES_V3_TX_TX_DRV_LVL);
>> +writel(pre_emphasis_cfg, tx + QSERDES_V3_TX_TX_EMP_POST1_LVL);
>> +writel(voltage_swing_cfg, tx2 + QSERDES_V3_TX_TX_DRV_LVL);
>> +writel(pre_emphasis_cfg, tx2 + QSERDES_V3_TX_TX_EMP_POST1_LVL);
>> +
>> +return 0;
>> +}
>> +
>> +static int qcs615_qmp_configure_dp_phy(struct qmp_usbc *qmp)
>> +{
>> +struct qmp_phy_dp_layout *layout = to_dp_layout(qmp);
>> +u32 status;
>> +
>> +writel(0x01, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +writel(0x05, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +writel(0x01, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +writel(0x09, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +
>> +writel(0x20, layout->dp_serdes + QSERDES_COM_RESETSM_CNTRL);
>> +
>> +// C_READY
>
> Use Linux coding style.
>
> Anyway, drop all useless comments. Say something useful or don't say
> anything.
>
Ok, will update in next seperated patches.
>> +if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_C_READY_STATUS,
>> +status,
>> +((status & BIT(0)) > 0),
>> +500,
>> +1)) {
>> +dev_err(qmp->dev, "C_READY not ready\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> +// FREQ_DONE
>> +if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_CMN_STATUS,
>> +status,
>> +((status & BIT(0)) > 0),
>> +500,
>> +1)){
>> +dev_err(qmp->dev, "FREQ_DONE not ready\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> +// PLL_LOCKED
>> +if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_CMN_STATUS,
>> +status,
>> +((status & BIT(1)) > 0),
>> +500,
>> +1)){
>> +dev_err(qmp->dev, "PLL_LOCKED not ready\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> +writel(0x19, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +udelay(10);
>> +
>> +// TSYNC_DONE
>> +if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
>> +status,
>> +((status & BIT(0)) > 0),
>> +500,
>> +1)){
>> +dev_err(qmp->dev, "TSYNC_DONE not ready\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> +// PHY_READY
>> +if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
>> +status,
>> +((status & BIT(1)) > 0),
>> +500,
>> +1)){
>> +dev_err(qmp->dev, "PHY_READY not ready\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> +writel(0x3f, layout->dp_tx + QSERDES_V3_TX_TRANSCEIVER_BIAS_EN);
>> +writel(0x10, layout->dp_tx + QSERDES_V3_TX_HIGHZ_DRVR_EN);
>> +writel(0x0a, layout->dp_tx + QSERDES_V3_TX_TX_POL_INV);
>> +writel(0x3f, layout->dp_tx2 + QSERDES_V3_TX_TRANSCEIVER_BIAS_EN);
>> +writel(0x10, layout->dp_tx2 + QSERDES_V3_TX_HIGHZ_DRVR_EN);
>> +writel(0x0a, layout->dp_tx2 + QSERDES_V3_TX_TX_POL_INV);
>> +
>> +writel(0x18, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +writel(0x19, layout->dp_phy + QSERDES_DP_PHY_CFG);
>> +
>> +if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
>> +status,
>> +((status & BIT(1)) > 0),
>> +500,
>> +1)){
>> +dev_err(qmp->dev, "PHY_READY not ready\n");
>> +return -ETIMEDOUT;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static int qcs615_qmp_calibrate_dp_phy(struct qmp_usbc *qmp)
>> +{
>> +static const u8 cfg1_settings[] = {0x13, 0x23, 0x1d};
>> +struct qmp_phy_dp_layout *layout = to_dp_layout(qmp);
>> +u8 val;
>> +
>> +layout->dp_aux_cfg++;
>> +layout->dp_aux_cfg %= ARRAY_SIZE(cfg1_settings);
>> +val = cfg1_settings[layout->dp_aux_cfg];
>> +
>> +
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Mon, Dec 02, 2024 at 06:31:44PM +0800, Xiangxu Yin wrote:
>
>
> On 11/29/2024 4:18 PM, Krzysztof Kozlowski wrote:
> > On 29/11/2024 08:57, Xiangxu Yin wrote:
> >> +static int qmp_usbc_com_init(struct phy *phy)
> >> {
> >>struct qmp_usbc *qmp = phy_get_drvdata(phy);
> >> - const struct qmp_phy_cfg *cfg = qmp->cfg;
> >> - void __iomem *pcs = qmp->pcs;
> >> + int num_vregs;
> >>u32 val = 0;
> >>int ret;
> >> + unsigned int reg_pwr_dn;
> >>
> >> - ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
> >> + if (qmp->type == QMP_PHY_USBC_USB) {
> >
> >
> > Sorry, all this code is unreviewable. Organize your changes in logical,
> > reviewable chunks.
> >
> Will create new patch list and seperate patchsets.
Please respond to the comment regarding the single PHY vs multiple PHYs
first.
> >> + struct qmp_phy_usb_cfg *cfg = to_usb_cfg(qmp);
> >> +
> >> + num_vregs = cfg->num_vregs;
> >> + reg_pwr_dn = cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL];
> >> + } else {
> >
--
With best wishes
Dmitry
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On Fri, 29 Nov 2024 at 09:59, Xiangxu Yin wrote:
>
> Extended DP support for QCS615 USB or DP phy. Differentiated between
> USBC and DP PHY using the match table’s type, dynamically generating
> different types of cfg and layout attributes during initialization based
> on this type. Static variables are stored in cfg, while parsed values
> are organized into the layout structure.
We didn't have an understanding / conclusion whether
qcom,usb-ssphy-qmp-usb3-or-dp PHYs are actually a single device / PHY
or two PHYs being placed next to each other. Could you please start
your commit message by explaining it? Or even better, make that a part
of the cover letter for a new series touching just the USBC PHY
driver. DP changes don't have anything in common with the PHY changes,
so you can split the series into two.
>
> Signed-off-by: Xiangxu Yin
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h |1 +
> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1453
>
Too many changes for a single patch. Please split into logical chunks.
> 2 files changed, 1254 insertions(+), 200 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> b/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> index
> 0ebd405bcaf0cac8215550bfc9b226f30cc43a59..59885616405f878885d0837838a0bac1899fb69f
> 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h
> @@ -25,6 +25,7 @@
> #define QSERDES_DP_PHY_AUX_CFG70x03c
> #define QSERDES_DP_PHY_AUX_CFG80x040
> #define QSERDES_DP_PHY_AUX_CFG90x044
> +#define QSERDES_DP_PHY_VCO_DIV 0x068
>
> /* QSERDES COM_BIAS_EN_CLKBUFLR_EN bits */
> # define QSERDES_V3_COM_BIAS_EN0x0001
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index
> cf12a6f12134dc77ff032f967b2efa43e3de4b21..7fece9d7dc959ed5a7c62077d8552324c3734859
> 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> @@ -22,13 +22,20 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include "phy-qcom-qmp-common.h"
>
> #include "phy-qcom-qmp.h"
> #include "phy-qcom-qmp-pcs-misc-v3.h"
>
> +#include "phy-qcom-qmp-dp-phy.h"
> +#include "phy-qcom-qmp-dp-phy-v3.h"
> +
> #define PHY_INIT_COMPLETE_TIMEOUT 1
> +#define SW_PORTSELECT_VAL BIT(0)
> +#define SW_PORTSELECT_MUX BIT(1)
>
> /* set of registers with offsets different per-PHY */
> enum qphy_reg_layout {
> @@ -284,7 +291,26 @@ static const struct qmp_phy_init_tbl
> qcm2290_usb3_pcs_tbl[] = {
> QMP_PHY_INIT_CFG(QPHY_V3_PCS_RX_SIGDET_LVL, 0x88),
> };
>
> -struct qmp_usbc_offsets {
> +enum qmp_phy_usbc_type {
> + QMP_PHY_USBC_INVALID,
How can a type be invalid?
> + QMP_PHY_USBC_USB,
> + QMP_PHY_USBC_DP,
> +};
> +
> +/* list of regulators */
> +struct qmp_regulator_data {
> + const char *name;
> + unsigned int enable_load;
> +};
> +
> +struct dev_cfg {
> + int type;
> + const void *cfg;
> +};
> +
> +struct qmp_usbc;
> +
> +struct qmp_usbc_usb_offsets {
> u16 serdes;
> u16 pcs;
> u16 pcs_misc;
> @@ -295,9 +321,9 @@ struct qmp_usbc_offsets {
> u16 rx2;
> };
>
> -/* struct qmp_phy_cfg - per-PHY initialization config */
> -struct qmp_phy_cfg {
> - const struct qmp_usbc_offsets *offsets;
> +/* struct qmp_phy_usb_cfg - per-usb PHY initialization config */
what is "per-usb PHY"?
> +struct qmp_phy_usb_cfg {
> + const struct qmp_usbc_usb_offsets *offsets;
>
> /* Init sequence for PHY blocks - serdes, tx, rx, pcs */
> const struct qmp_phy_init_tbl *serdes_tbl;
> @@ -317,11 +343,7 @@ struct qmp_phy_cfg {
> const unsigned int *regs;
> };
>
> -struct qmp_usbc {
> - struct device *dev;
> -
> - const struct qmp_phy_cfg *cfg;
> -
> +struct qmp_phy_usb_layout {
> void __iomem *serdes;
> void __iomem *pcs;
> void __iomem *pcs_misc;
> @@ -329,28 +351,67 @@ struct qmp_usbc {
> void __iomem *rx;
> void __iomem *tx2;
> void __iomem *rx2;
> -
> struct regmap *tcsr_map;
> u32 vls_clamp_reg;
> -
> + enum phy_mode mode;
> + struct typec_switch_dev *sw;
> struct clk *pipe_clk;
> + struct clk_fixed_rate pipe_clk_fixed;
> +};
> +
> +struct qmp_usbc_dp_offsets {
> + u16 dp_serdes;
> + u16 dp_txa;
> + u16 dp_txb;
> + u16 dp_phy;
> +};
> +
> +/* struct qmp_phy_dp_cfg - per-dp PHY initialization config */
> +struct qmp_phy_dp_cfg {
> + const struct qmp_usbc_dp_offsets *offsets;
> +
> + /* DP PHY swing and pre_emphasis tables */
> + const u8 (*swing_tbl)[4][4];
> + const u8 (*pre_emphasis_tbl)[4][4];
> +
> + // /* DP P
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
Hi Xiangxu, kernel test robot noticed the following build warnings: [auto build test WARNING on f486c8aa16b8172f63bddc70116a0c897a7f3f02] url: https://github.com/intel-lab-lkp/linux/commits/Xiangxu-Yin/dt-bindings-display-msm-Document-DP-on-QCS615/20241129-160612 base: f486c8aa16b8172f63bddc70116a0c897a7f3f02 patch link: https://lore.kernel.org/r/20241129-add-displayport-support-for-qcs615-platform-v1-3-09a4338d93ef%40quicinc.com patch subject: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615 config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241129/[email protected]/config) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241129/[email protected]/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/ All warnings (new ones prefixed by >>): In file included from drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:17: In file included from include/linux/phy/phy.h:17: In file included from include/linux/regulator/consumer.h:35: In file included from include/linux/suspend.h:5: In file included from include/linux/swap.h:9: In file included from include/linux/memcontrol.h:21: In file included from include/linux/mm.h:2223: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion] 504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS + |~ ^ 505 |item]; | include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion] 511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS + |~ ^ 512 |NR_VM_NUMA_EVENT_ITEMS + |~~ include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion] 518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_" | ~~~ ^ ~~~ include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion] 524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS + |~ ^ 525 |NR_VM_NUMA_EVENT_ITEMS + |~~ >> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:721:24: warning: variable >> 'pre_emphasis_cfg' is uninitialized when used here [-Wuninitialized] 721 | if ((v_level > 4) || (pre_emphasis_cfg > 4)) { | ^~~~ drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:708:40: note: initialize the variable 'pre_emphasis_cfg' to silence this warning 708 | u8 voltage_swing_cfg, pre_emphasis_cfg; | ^ |= '\0' >> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:1801:47: warning: variable 'ret' is >> uninitialized when used here [-Wuninitialized] 1801 | dev_err(dev, "get resource fail, ret:%d\n", ret); | ^~~ include/linux/dev_printk.h:154:65: note: expanded from macro 'dev_err' 154 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~ include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap' 110 | _p_func(dev, fmt, ##__VA_ARGS__); \ | ^~~ drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:1797:9: note: initialize the variable 'ret' to silence this warning 1797 | int ret; |^ | = 0 >> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:2082:13: warning: variable 'np' is >> used uninitialized whenever 'if' condition is false >> [-Wsometimes-uninitialized] 2082 | } else if (qmp->type == QMP_PHY_USBC_DP) { |^~~~ drivers/phy/qualcomm/phy-qcom-qmp-usbc.c:2150:14: note: u
Re: [PATCH 3/8] phy: qcom: qmp-usbc: Add DP phy mode support on QCS615
On 29/11/2024 08:57, Xiangxu Yin wrote:
> Extended DP support for QCS615 USB or DP phy. Differentiated between
> USBC and DP PHY using the match table’s type, dynamically generating
> different types of cfg and layout attributes during initialization based
> on this type. Static variables are stored in cfg, while parsed values
> are organized into the layout structure.
>
> Signed-off-by: Xiangxu Yin
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-dp-phy.h |1 +
> drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 1453
>
> 2 files changed, 1254 insertions(+), 200 deletions(-)
...
> + /* program default setting first */
> + writel(0x2A, tx + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(0x20, tx + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> + writel(0x2A, tx2 + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(0x20, tx2 + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> +
> + writel(voltage_swing_cfg, tx + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(pre_emphasis_cfg, tx + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> + writel(voltage_swing_cfg, tx2 + QSERDES_V3_TX_TX_DRV_LVL);
> + writel(pre_emphasis_cfg, tx2 + QSERDES_V3_TX_TX_EMP_POST1_LVL);
> +
> + return 0;
> +}
> +
> +static int qcs615_qmp_configure_dp_phy(struct qmp_usbc *qmp)
> +{
> + struct qmp_phy_dp_layout *layout = to_dp_layout(qmp);
> + u32 status;
> +
> + writel(0x01, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x05, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x01, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x09, layout->dp_phy + QSERDES_DP_PHY_CFG);
> +
> + writel(0x20, layout->dp_serdes + QSERDES_COM_RESETSM_CNTRL);
> +
> + // C_READY
Use Linux coding style.
Anyway, drop all useless comments. Say something useful or don't say
anything.
> + if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_C_READY_STATUS,
> + status,
> + ((status & BIT(0)) > 0),
> + 500,
> + 1)) {
> + dev_err(qmp->dev, "C_READY not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + // FREQ_DONE
> + if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_CMN_STATUS,
> + status,
> + ((status & BIT(0)) > 0),
> + 500,
> + 1)){
> + dev_err(qmp->dev, "FREQ_DONE not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + // PLL_LOCKED
> + if (readl_poll_timeout(layout->dp_serdes + QSERDES_COM_CMN_STATUS,
> + status,
> + ((status & BIT(1)) > 0),
> + 500,
> + 1)){
> + dev_err(qmp->dev, "PLL_LOCKED not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + writel(0x19, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + udelay(10);
> +
> + // TSYNC_DONE
> + if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
> + status,
> + ((status & BIT(0)) > 0),
> + 500,
> + 1)){
> + dev_err(qmp->dev, "TSYNC_DONE not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + // PHY_READY
> + if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
> + status,
> + ((status & BIT(1)) > 0),
> + 500,
> + 1)){
> + dev_err(qmp->dev, "PHY_READY not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + writel(0x3f, layout->dp_tx + QSERDES_V3_TX_TRANSCEIVER_BIAS_EN);
> + writel(0x10, layout->dp_tx + QSERDES_V3_TX_HIGHZ_DRVR_EN);
> + writel(0x0a, layout->dp_tx + QSERDES_V3_TX_TX_POL_INV);
> + writel(0x3f, layout->dp_tx2 + QSERDES_V3_TX_TRANSCEIVER_BIAS_EN);
> + writel(0x10, layout->dp_tx2 + QSERDES_V3_TX_HIGHZ_DRVR_EN);
> + writel(0x0a, layout->dp_tx2 + QSERDES_V3_TX_TX_POL_INV);
> +
> + writel(0x18, layout->dp_phy + QSERDES_DP_PHY_CFG);
> + writel(0x19, layout->dp_phy + QSERDES_DP_PHY_CFG);
> +
> + if (readl_poll_timeout(layout->dp_phy + QSERDES_V3_DP_PHY_STATUS,
> + status,
> + ((status & BIT(1)) > 0),
> + 500,
> + 1)){
> + dev_err(qmp->dev, "PHY_READY not ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static int qcs615_qmp_calibrate_dp_phy(struct qmp_usbc *qmp)
> +{
> + static const u8 cfg1_settings[] = {0x13, 0x23, 0x1d};
> + struct qmp_phy_dp_layout *layout = to_dp_layout(qmp);
> + u8 val;
> +
> + layout->dp_aux_cfg++;
> + layout->dp_aux_cfg %= ARRAY_SIZE(cfg1_settings);
> + val = cfg1_settings[layout->dp_aux_cfg];
> +
> + writel(val, layout->dp_phy + QSERDES_DP_PHY_AUX_CFG1);
> +
> + qmp_usbc_check_dp_phy(qmp, "pos_calibrate");
> +
> + return 0;
> +}
> +
> +stati
