On Wed, Nov 16, 2022 at 01:17:54PM -0700, Alex Helms wrote: > On 11/16/2022 1:50 AM, Krzysztof Kozlowski wrote: > > On 16/11/2022 00:37, Alex Helms wrote: > >> Add dt bindings for the Renesas ProXO oscillator. > >> > >> Signed-off-by: Alex Helms <alexander.helms.jy@xxxxxxxxxxx> > >> --- > >> .../bindings/clock/renesas,proxo.yaml | 51 +++++++++++++++++++ > >> MAINTAINERS | 5 ++ > >> 2 files changed, 56 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/clock/renesas,proxo.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/clock/renesas,proxo.yaml b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml > >> new file mode 100644 > >> index 000000000..ff960196d > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/clock/renesas,proxo.yaml > >> @@ -0,0 +1,51 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Frenesas%2Cproxo.yaml%23&data=05%7C01%7Calexander.helms.jy%40renesas.com%7C248dc84dbca44a4013d408dac7af9cf1%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041854305996374%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=iGbtWJLjV%2FM%2Fps0lPk7f40bMzX8qdt8VZBtH9J4LdOw%3D&reserved=0 > >> +$schema: https://jpn01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7Calexander.helms.jy%40renesas.com%7C248dc84dbca44a4013d408dac7af9cf1%7C53d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C638041854305996374%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zYh4aHuw6G6A35rXBD7FTKeFrC7Hfcxag60ghkKUaGA%3D&reserved=0 > >> + > >> +title: Renesas ProXO Oscillator Device Tree Bindings > > > > Same comments as for your other patch. All the same... > > > >> + > >> +maintainers: > >> + - Alex Helms <alexander.helms.jy@xxxxxxxxxxx> > >> + > >> +description: > >> + Renesas ProXO is a family of programmable ultra-low phase noise > >> + quartz-based oscillators. > >> + > >> +properties: > >> + '#clock-cells': > >> + const: 0 > >> + > >> + compatible: > >> + enum: > >> + - renesas,proxo-xp > >> + > >> + reg: > >> + maxItems: 1 > >> + > >> + clock-output-names: > >> + maxItems: 1 > >> + > >> + renesas,crystal-frequency-hz: > >> + description: Internal crystal frequency, default is 50000000 (50MHz) > > > > If it is internal, then it is fixed, right? Embedded in the chip, always > > the same. Why do you need to specify it? > > > > Yes, it is embedded in the package but there are different values > depending on what chip is ordered and therefore must be specified for > some configurations. > > I'm also not sure what you mean by me ignoring Rob's comment. I > explained my case for calling it "crystal-frequency-hz" and moved > forward. I can call it "clock-frequency" if you want but I find that > more confusing. Yes it is a built-in name in the schema but it seems to > be used in a variety of ways. Some devices use it as a crystal input, > but most seem to use it as the desired output frequency of the device > which is not how it is used here. Therefore I chose a more clear name > that better reflects what it is doing. I think it is fine as-is. But you should have 'default: 50000000' instead of prose. Rob
