Re: [PATCH] dt-bindings: phy: imx8mq-usb: optionally refer to usb-switch.yaml

Xu Yang
2024-11-19T10:09:56Z

On Fri, Nov 15, 2024 at 06:34:50PM +0000, Conor Dooley wrote:
> On Fri, Nov 15, 2024 at 10:00:45AM +0800, Xu Yang wrote:
> > On Thu, Nov 14, 2024 at 05:45:07PM +0000, Conor Dooley wrote:
> > > On Thu, Nov 14, 2024 at 06:22:03PM +0800, Xu Yang wrote:
> > > > The i.MX95 usb-phy can work with or without orientation-switch. With
> > > > current setting, if usb-phy works without orientation-switch, the
> > > > dt-schema check will show below error:
> > > >
> > > > phy@4c1f0040: 'oneOf' conditional failed, one must be fixed:
> > > > 'port' is a required property
> > > > 'ports' is a required property
> > > > from schema $id: http://devicetree.org/schemas/phy/fsl,imx8mq-usb-phy.yaml#
> > > >
> > > > This will add a condition to optionally refer to usb-switch.yaml.
> > > >
> > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > >
> > > $subject is not what the patch does.
> >
> > I look through other yaml and find below format could achieve the
> > restriction on one property:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/tree/Documentation/devicetree/bindings/usb/gpio-sbu-mux.yaml?h=usb-testing#n54
> >
> > - if:
> > required:
> > - mode-switch
> > then:
> > required:
> > - enable-gpios
> >
> > If mode-switch is present, then enable-gpios is required. If not, then
> > enable-gpios is not required.
> >
> > For my case, if compatible contains "fsl,imx95-usb-phy" and orientation-switch
> > is present, then this schema needs to refer to usb-switch.yaml. If not. then
> > this schema will not refer to usb-switch.yaml. So the subject could reflect
> > the thing this patch does.
> >
> > Is this feasible? If not, could you give some advices?
>
> Hmm, I guess I can now see an interpretation of the wording that
> reflects what the patch contents contains. However, you need a hardware
> based justification for the condition you're adding since it disables
> mode-switch and defining port nodes etc if orientation-switch isn't present.

Okay. You are right. Based on current schema, it does disable mode-switch and
port. However, the initial intention of referring usb-switch.yaml is only to
support orientation switching.

> That's fine if that hardware doesn't support mode switching or ports
> without orientation switching, but not if it does.

Yes, the hardware doesn't support mode swiching for now. Therefore, it should
be fine to add such condition. Maybe the future hardware will need mode-switch
and this schema needs update accordingly.

I find the condition of judging compatible "fsl,imx95-usb-phy" and orientation-switch
present is a little complicated. I plan to send v2 to simply the condition.

Thanks,
Xu Yang

>
> Cheers,
> Conor.
>
> >
> > Thanks,
> > Xu Yang
> >
> > >
> > > > ---
> > > > Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > > > index 6d6d211883ae..1238792157f8 100644
> > > > --- a/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > > > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-usb-phy.yaml
> > > > @@ -118,6 +118,8 @@ allOf:
> > > > contains:
> > > > enum:
> > > > - fsl,imx95-usb-phy
> > > > + required:
> > > > + - orientation-switch
> > > > then:
> > > > $ref: /schemas/usb/usb-switch.yaml#
> > > >
> > > > --
> > > > 2.34.1
> > > >
> >
> >