devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: <markus.stockhausen@gmx.de>
To: "'Krzysztof Kozlowski'" <krzk@kernel.org>,
	<linux-phy@lists.infradead.org>,
	<chris.packham@alliedtelesis.co.nz>, <devicetree@vger.kernel.org>
Subject: AW: [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding
Date: Wed, 16 Oct 2024 17:30:11 +0200	[thread overview]
Message-ID: <002201db1fe0$4b398dc0$e1aca940$@gmx.de> (raw)
In-Reply-To: <e0355f2b-9d77-4792-9405-14b0bf79ac32@kernel.org>

Hi Krzysztof,

with your feedback on the latest version I will take up the issues from 
v2 once again. To be sure that I do not miss anything in upcoming v5 
I will comment on all your feedback. 

> > ....
> > Changes in v2:
> > - new subject
> > - removed patch command sequences
> > - renamed parameter controlled-ports to realtek,controlled-ports
>
> Changelog goes under ---.

After reading this another 4 times now I think I understand. You mean
"put changelog below signed-off-by". Will do with next patch.

> > ....
> > diff --git 
> > a/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml 
> > b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
> > new file mode 100644
> > index 000000000000..a72ac206b35f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/realtek,otto-serdes.yaml
>
> Nothing improved.

In between renamed to compatible "realtek,rtl8380m-serdes.yaml". I hope
that fits the requested naming convention.

> > +  The driver exposes the SerDes registers different from the hardware 
> > + but instead gives a  consistent view and programming interface. So 
> > + the RTL838x series has 6 ports and 4 pages, the  RTL839x has 14 
> > + ports and 12 pages, the RTL930x has 12 ports and 64 pages and the 
> > + RTL931x has
> > +  14 ports and 192 pages.
>
> Totally messed wrapping. Please wrap your code as Linux coding style.

Was restyled in between. If this is still an issue in latest version, please advise.

> > +  reg:
> > +    items:
> > +      description:
> > +        The primary SerDes paged register memory location. Other SerDes control and management
> > +        registers are distributed all over the I/O memory space and are identified by the driver.
>
> What happened here? I asked only about |. Why are you adding unrelated changes?
>
> Anyway, still not tested and still does not look any other binding.

Has been tested in between with "make dt_binding_check".

> > +  realtek,controlled-ports:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      A bit mask defining the ports that are actively controlled by 
> > + the driver. In case a bit is
>
> Driver? Bindings are not about drivers. Drop.
>
> I don't think you implemented my feedback.

All these have been removed.

> > +additionalProperties:
> > +  false
>
> Please open any existing binding and do it like there. Or start from scratch from example-schema.

Was converted to one line.

> > +
> > +examples:
> > +  - |
> > +    serdes: serdes@1b00e780 {
> > +      compatible = "realtek,rtl8380-serdes", "realtek,otto-serdes";
> > +      reg = <0x1b00e780 0x1200>;
> > +      controlled-ports = <0x003f>;
> > +      #phy-cells = <4>;
> > +    };
>
> One example is enough.

Only one example left.

Best regards.

Markus


  parent reply	other threads:[~2024-10-16 15:30 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-07 16:36 [PATCH v2 0/3] phy: Realtek Otto SerDes: add new driver Markus Stockhausen
2024-10-07 16:36 ` [PATCH v2 1/3] dt-bindings: phy: add realtek,otto-serdes PHY binding Markus Stockhausen
2024-10-07 18:17   ` Rob Herring (Arm)
2024-10-07 19:26   ` Krzysztof Kozlowski
2024-10-08  5:38     ` AW: " markus.stockhausen
2024-10-08  6:17       ` Krzysztof Kozlowski
2024-10-08  6:56         ` AW: " markus.stockhausen
2024-10-08  8:32           ` Krzysztof Kozlowski
2024-10-08  9:27             ` AW: " markus.stockhausen
2024-10-16 15:30     ` markus.stockhausen [this message]
2024-10-17  6:15       ` Krzysztof Kozlowski
2024-10-07 19:30   ` Rob Herring
2024-10-08 12:27     ` AW: " markus.stockhausen
2024-10-08  7:04   ` Krzysztof Kozlowski
2024-10-08  7:06   ` Krzysztof Kozlowski
2024-10-07 16:36 ` [PATCH v2 2/3] phy: Realtek Otto SerDes driver Markus Stockhausen
2024-10-07 19:32   ` Krzysztof Kozlowski
2024-10-07 16:36 ` [PATCH v2 3/3] phy: Integrate Realtek Otto SerDes driver into build system Markus Stockhausen
2024-10-07 19:27   ` Krzysztof Kozlowski
2024-10-08  6:38   ` kernel test robot
2024-10-08  7:20   ` kernel test robot
2024-10-08  8:21   ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='002201db1fe0$4b398dc0$e1aca940$@gmx.de' \
    --to=markus.stockhausen@gmx.de \
    --cc=chris.packham@alliedtelesis.co.nz \
    --cc=devicetree@vger.kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-phy@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).