Message ID | 20250321135854.1431375-1-ivo.ivanov.ivanov1@gmail.com |
---|---|
Headers | show |
Series | phy: samsung: add Exynos2200 SNPS eUSB2 driver | expand |
On 3/21/25 16:15, Dmitry Baryshkov wrote: > On Fri, Mar 21, 2025 at 03:58:47PM +0200, Ivaylo Ivanov wrote: >> As not only Qualcomm, but also Samsung is using the Synopsys eUSB2 IP >> (albeit with a different register layout) in their newer SoCs, move the >> driver out of its vendor sub-directory and rename it to phy-snps-eusb2. >> >> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> >> Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Please fix the order of the tags. SoB should come after Suggested-by > (and usually it is the last one). Hm? I haven't had issues with SoB at the top in the past. > >> --- >> drivers/phy/Kconfig | 8 ++++++++ >> drivers/phy/Makefile | 1 + >> .../{qualcomm/phy-qcom-snps-eusb2.c => phy-snps-eusb2.c} | 0 >> drivers/phy/qualcomm/Kconfig | 9 --------- >> drivers/phy/qualcomm/Makefile | 1 - >> 5 files changed, 9 insertions(+), 10 deletions(-) >> rename drivers/phy/{qualcomm/phy-qcom-snps-eusb2.c => phy-snps-eusb2.c} (100%) > With that fixed: > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> Thanks! Best regards, Ivaylo > >
On Fri, Mar 21, 2025 at 03:58:49PM +0200, Ivaylo Ivanov wrote: > The current phy init consists of hardware power-up, as well as > QCOM-specific eUSB2 init code. Split it into two parts, to make room > for such non-QCOM init code. > > Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> > --- > drivers/phy/phy-snps-eusb2.c | 96 +++++++++++++++++++++++------------- > 1 file changed, 63 insertions(+), 33 deletions(-) > > @@ -378,6 +401,11 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev) > if (!phy) > return -ENOMEM; > > + drv_data = of_device_get_match_data(dev); > + if (!drv_data) > + return -EINVAL; > + phy->data = drv_data; Nit and completely bikeshedding, but this looks simpler: phy->data = of_device_get_match_data(dev); if (!phy->data) return -EINVAL; Anyway: Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> > + > phy->base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(phy->base)) > return PTR_ERR(phy->base); > @@ -424,8 +452,10 @@ static int snps_eusb2_hsphy_probe(struct platform_device *pdev) > } > > static const struct of_device_id snps_eusb2_hsphy_of_match_table[] = { > - { .compatible = "qcom,sm8550-snps-eusb2-phy", }, > - { }, > + { > + .compatible = "qcom,sm8550-snps-eusb2-phy", > + .data = &sm8550_snps_eusb2_phy, > + }, { }, > }; > MODULE_DEVICE_TABLE(of, snps_eusb2_hsphy_of_match_table); > > -- > 2.43.0 >
On Fri, Mar 21, 2025 at 03:58:53PM +0200, Ivaylo Ivanov wrote: > The Exynos2200 SoC reuses the Synopsis eUSB2 PHY IP, alongside an > external repeater, for USB 2.0. Add support for it to the existing > driver, while keeping in mind that it requires enabled more than the > reference clock. > > Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com> > --- > drivers/phy/Kconfig | 2 +- > drivers/phy/phy-snps-eusb2.c | 162 ++++++++++++++++++++++++++++++++++- > 2 files changed, 159 insertions(+), 5 deletions(-) > Acked-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
On Fri, Mar 21, 2025 at 03:58:45PM +0200, Ivaylo Ivanov wrote: > description: > - eUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets. > + eUSB2 controller supports LS/FS/HS usb connectivity. > > properties: > compatible: > @@ -23,6 +23,7 @@ properties: > - qcom,x1e80100-snps-eusb2-phy > - const: qcom,sm8550-snps-eusb2-phy > - const: qcom,sm8550-snps-eusb2-phy > + - const: samsung,exynos2200-snps-eusb2-phy These two entries is just an enum. > > reg: > maxItems: 1 > @@ -31,12 +32,12 @@ properties: > const: 0 > > clocks: > - items: > - - description: ref > + minItems: 1 > + maxItems: 3 I am still not conviced that creating one schema for these devices brings benefits. If this is going to be one binding, then keep the list here with three items and add minItems, so the list is the same for all variants. > > clock-names: > - items: > - - const: ref > + minItems: 1 > + maxItems: 3 Keep the list here with three items and add minItems. > > resets: > maxItems: 1 > @@ -62,7 +63,52 @@ required: > - clock-names > - vdd-supply > - vdda12-supply > - - resets > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - qcom,sm8550-snps-eusb2-phy > + > + then: > + properties: > + reg: > + maxItems: 1 Not much improved, my comment is still valid. > + > + clocks: > + items: > + - description: ref maxItems: 1 > + > + clock-names: > + items: > + - const: ref maxItems: 1 > + > + required: > + - resets > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - samsung,exynos2200-snps-eusb2-phy > + > + then: > + properties: > + Drop blank line. and the clocks get here minItems: 3 Best regards, Krzysztof