Re: [PATCH v2 2/2] phy: dw-dphy-rx: Add dt bindings for Synopsys MIPI D-PHY RX

["Poduval, Karthik" <kpoduval@lab126.com>]

Hi Krzysztof,

Thanks for your comments.

On Thu, 2025-07-10 at 09:24 +0200, Krzysztof Kozlowski wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> 
> 
> 
> On Wed, Jul 09, 2025 at 07:42:21PM -0700, Karthik Poduval wrote:
> > +---
> > +$id: http://devicetree.org/schemas/phy/snps,dw-dphy-rx.yaml#
> 
> Filename and id should match compatible.
> 
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys Designware MIPI D-PHY RX
> > +
> > +maintainers:
> > +  - Karthik Poduval <kpoduval@lab126.com>
> > +  - Jason Xiong <jyxiong@amazon.com>
> > +  - Miguel Lopes <miguel.lopes@synopsys.com
> > +
> > +description: |
> > +  These are the bindings for Synopsys Designware MIPI DPHY RX phy
> > driver.
> > +  Currently only supported phy version is v1.2.
> > +
> > +properties:
> > +  compatible:
> > +    const: snps,dw-dphy-1p2
> 
> You should rather use SoC compatibles.
If similar Synopsys D-PHY is used in multiple SoCs with slight
variations, should each SoC have its own D-PHY driver or is it better
to have a common one that works for all SoCs like arm,smmu-v3 or like
the EEPROM driver below ? 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/misc/eeprom/eeprom_93xx46.c?h=v6.16-rc5#n391
where driver specific driver_data could support multiple versions or
SoC variations. 
Our team thought of the common driver approach however looking for
general guidance from PHY subsystem maintainers and contributors.
> 
> > +
> > +  '#phy-cells':
> > +    const: 0
> > +
> > +  reg:
> > +    minItems: 2
> > +    maxItems: 2
> 
> List the items instead
> 
> And fix the order, reg goes second.
ack
> 
> > +
> > +required:
> > +  - compatible
> > +  - '#phy-cells'
> > +  - reg
> 
> Here as well
ack
> 
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    dw_dphy_rx: dw-dphy@900000040 {
> 
> phy@
> 
> > +        compatible = "snps,dw-dphy-1p2";
> > +        #phy-cells = <0>;
> > +        reg = <0x0 0x90000040 0x0 0x20>, <0x0 0x90001000 0x0 0x8>;
> > +        status = "disabled";
> 
> No, drop. And fix the order of properties, see DTS coding style.
> 
> > +    };
> 
> Best regards,
> Krzysztof
>