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
next prev 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).