On 16/05/2023 11:41, Julien Stephan wrote: > On Tue, May 16, 2023 at 10:07:47AM +0200, Krzysztof Kozlowski wrote: >> On 15/05/2023 11:05, Julien Stephan wrote: >>> From: Florian Sylvestre <fsylvestre@xxxxxxxxxxxx> >>> >>> This adds the bindings, for the MIPI CD-PHY module v 0.5 embedded in >>> some Mediatek soc, such as the mt8365 >>> >>> Signed-off-by: Florian Sylvestre <fsylvestre@xxxxxxxxxxxx> >>> Signed-off-by: Julien Stephan <jstephan@xxxxxxxxxxxx> >> >> What are the changes? IOW: changelog here or in cover letter. >> > Hi Krzysztof, > I added a changelog in the cover letter, but I will try to be more > descritpive next time. Changes from v1 are mainly style issues fixed > (mostly from your first review) What do you mean by "in cover letter"? There is no cover letter. > >> Subject: you have some multiple spaces. >> >> Subject: drop driver. Bindings are not for drivers. >> >>> --- >>> .../phy/mediatek,phy-mipi-csi-0-5.yaml | 62 +++++++++++++++++++ >>> MAINTAINERS | 6 ++ >>> 2 files changed, 68 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/phy/mediatek,phy-mipi-csi-0-5.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,phy-mipi-csi-0-5.yaml b/Documentation/devicetree/bindings/phy/mediatek,phy-mipi-csi-0-5.yaml >>> new file mode 100644 >>> index 000000000000..5aa8c0b41cdf >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/phy/mediatek,phy-mipi-csi-0-5.yaml >>> @@ -0,0 +1,62 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-Only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/phy/mediatek,phy-mipi-csi-0-5.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Mediatek Sensor Interface MIPI CSI CD-PHY >>> + >>> +maintainers: >>> + - Julien Stephan <jstephan@xxxxxxxxxxxx> >>> + - Andy Hsieh <andy.hsieh@xxxxxxxxxxxx> >>> + >>> +description: >>> + The SENINF CD-PHY is a set of CD-PHY connected to the SENINF CSI-2 >>> + receivers. The number of PHYs depends on the SoC model. >>> + Depending on the soc model, each PHYs can support CDPHY or DPHY only >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - mediatek,phy-mipi-csi-0-5 >> >> SoC based compatibles. 0-5 is odd. >> >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + '#phy-cells': >>> + const: 0 >>> + >>> + mediatek,is_cdphy: >> >> No underscores in node names. >> >>> + description: >>> + Specify if the current phy support CDPHY configuration >> >> Why this cannot be implied from compatible? Add specific compatibles. >> >> > This cannot be implied by compatible because the number of phys depends > on the soc and each phy can be either D-PHY only or CD-PHY capable. > For example mt8365 has 2 phy: CSI0 and CSI1. CSI1 is DPHY only and CSI0 is CD-PHY So it is SoC specific so why it cannot be implied by compatible? I don't understand. You will have SoC specific compatibles, right? or you just ignored my comments here? >> Best regards, Krzysztof
