Message-ID: <614fa099-e666-03da-1b11-29cc804bf847@linaro.org>
Date:   Mon, 20 Mar 2023 18:28:22 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [EXT] Re: [PATCH v2 1/3] dt-bindings: usb: cdns-imx8qm: add
 imx8qm cdns3 glue bindings
Content-Language: en-US
To:     Frank Li <frank.li@nxp.com>,
        "shawnguo@kernel.org" <shawnguo@kernel.org>
Cc:     "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "festevam@gmail.com" <festevam@gmail.com>,
        "imx@lists.linux.dev" <imx@lists.linux.dev>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        dl-linux-imx <linux-imx@nxp.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "robh+dt@kernel.org" <robh+dt@kernel.org>,
        "s.hauer@pengutronix.de" <s.hauer@pengutronix.de>
References: <20230316212712.2426542-1-Frank.Li@nxp.com>
 <20230316212712.2426542-2-Frank.Li@nxp.com>
 <e6935c0c-375e-b763-ea91-3b8bbc906ebc@linaro.org>
 <AM6PR04MB4838D1958A029701E1601BA588BD9@AM6PR04MB4838.eurprd04.prod.outlook.com>
 <1fd1fe42-3da6-1598-a04d-cb99a9b4b145@linaro.org>
 <AM6PR04MB483800D7CDCC7AF48F88BF9688809@AM6PR04MB4838.eurprd04.prod.outlook.com>
 <b8801c83-f2dc-f144-de58-03e5c24436fd@linaro.org>
 <AM6PR04MB4838F5C9EF13A588E799D5C488809@AM6PR04MB4838.eurprd04.prod.outlook.com>
 <fc5c76d1-51cd-5992-9bfa-06f57874fc03@linaro.org>
 <AM6PR04MB4838C6B05F46AD94153DDC8388809@AM6PR04MB4838.eurprd04.prod.outlook.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <AM6PR04MB4838C6B05F46AD94153DDC8388809@AM6PR04MB4838.eurprd04.prod.outlook.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 20/03/2023 18:02, Frank Li wrote:

>>> Although frequency is fixed, clock name may change for difference
>> platform.
>>>
>>>                 assigned-clocks = <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_PER>,
>>>                                            <&clk IMX_SC_R_USB_2 IMX_SC_PM_CLK_MISC>,
>>>                                            <&clk IMX_SC_R_USB_2
>> IMX_SC_PM_CLK_MST_BUS>;
>>>                assigned-clock-rates = <125000000>, <12000000>, <250000000>;
>>>
>>> some platform use IMX_SC_R_USB_2, other platform may use
>> IMX_SC_R_USB_3.
>>
>> This I understand, you wrote it above, so nothing new and my concerns
>> are still there.
> 
> I think Fixed value is not good reason. All reg base address, irq number are all for fixed number. The same

No, because one device - IP block - could have different addresses,
depending how it is wired/implemented in given SoC. Also our
representation of devices in the kernel requires regs/interrupts coming
from DTS, thus DTS is also answer to entire design of kernel and other SW.

That's not the case here at all.

> Logic can be applied to irq-provider driver. But why still be descript in dts? It is hardware property.    
> 
> https://elixir.bootlin.com/linux/v4.8/source/Documentation/devicetree/bindings/clock/clock-bindings.txt
> have not said that can't set to fixed clock frequency. 

I don't understand this.

> 
> This is quick common case for network, USB, SATA, PCIE,  which protocol defined
> Frequency.  

And they do not define fixed values in the bindings, so? There are
exceptions, but it's usually not argument, right?

> 
> https://elixir.bootlin.com/linux/v6.3-rc3/source/Documentation/devicetree/bindings/ata/qcom-sata.txt
> https://elixir.bootlin.com/linux/v6.3-rc3/source/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml

The second is a good example - as you can see, there is a choice of
values, so they are not exactly fixed.

> 
> Such frequency information is necessary.  We can put to dts or clock drivers.  The clock driver

If this is the argument, then the answer is NAK. Sorry, but DTS is not
for offloading fixed stuff just because you do not want to work on
drivers. The same for discoverable stuff.

> Become bigger, or dts become bigger.  I think the key point is if property to descript hardware information.  

You have to understand that with your binding you are not allowing to
any changes of these frequencies.

Best regards,
Krzysztof