| Message ID | 54ee114c08f35ab8b5dc584fd76135ac9076f5a7.1693218539.git.zhoubinbin@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | soc: loongson: Fix some issues about loongson_pm2 | expand |
On 29/08/2023 05:21, Binbin Zhou wrote: > HI Conor: > > Thanks for your reply. > > On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote: >> >> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote: >>> Document the Power Management Unit system controller compatible for >>> Loongson-2K2000. >>> >>> This is a missing compatible, now we add it. >>> >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> >>> --- >>> .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml >>> index da2dcfeebf12..7473c5659929 100644 >>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml >>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml >>> @@ -15,6 +15,7 @@ properties: >>> - enum: >>> - loongson,ls2k0500-pmc >>> - loongson,ls2k1000-pmc >>> + - loongson,ls2k2000-pmc >> >> Same thing here as the driver patch, the pmc on this newly added SoC >> appears to use the same codepaths as the existing ones. Does it share a >> programming model & should there be a fallback compatible here? > > I noticed the guideline about fallback compatible: > > "DO use fallback compatibles when devices are the same as or a subset > of prior implementations." > > But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset. We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of them. If they are independent, why would they use the same interface? > Can we define a "loongson,ls2k-pmc" superset for each ls2k SoC > compatible fallback. > > Such as: > > compatible: > oneOf: > - enum: > - loongson,ls2k0500-pmc > - loongson,ls2k1000-pmc > - loongson,ls2k2000-pmc > - const: loongson,ls2k-pmc This is discouraged. Use 0500 as fallback. Best regards, Krzysztof
On Tue, Aug 29, 2023 at 08:29:43AM +0200, Krzysztof Kozlowski wrote: > On 29/08/2023 05:21, Binbin Zhou wrote: > > HI Conor: > > > > Thanks for your reply. > > > > On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote: > >> > >> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote: > >>> Document the Power Management Unit system controller compatible for > >>> Loongson-2K2000. > >>> > >>> This is a missing compatible, now we add it. > >>> > >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > >>> --- > >>> .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml | 1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml > >>> index da2dcfeebf12..7473c5659929 100644 > >>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml > >>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml > >>> @@ -15,6 +15,7 @@ properties: > >>> - enum: > >>> - loongson,ls2k0500-pmc > >>> - loongson,ls2k1000-pmc > >>> + - loongson,ls2k2000-pmc > >> > >> Same thing here as the driver patch, the pmc on this newly added SoC > >> appears to use the same codepaths as the existing ones. Does it share a > >> programming model & should there be a fallback compatible here? > > > > I noticed the guideline about fallback compatible: > > > > "DO use fallback compatibles when devices are the same as or a subset > > of prior implementations." > > > > But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset. > > We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of > them. If they are independent, why would they use the same interface? > > > Can we define a "loongson,ls2k-pmc" superset for each ls2k SoC > > compatible fallback. > > > > Such as: > > > > compatible: > > oneOf: > > - enum: > > - loongson,ls2k0500-pmc > > - loongson,ls2k1000-pmc > > - loongson,ls2k2000-pmc > > - const: loongson,ls2k-pmc > > This is discouraged. Use 0500 as fallback. The "code" here is also invalid, the oneOf would been to be items.
On Tue, Aug 29, 2023 at 02:52:48PM +0800, Binbin Zhou wrote: > On Tue, Aug 29, 2023 at 2:29 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: > > > > On 29/08/2023 05:21, Binbin Zhou wrote: > > > HI Conor: > > > > > > Thanks for your reply. > > > > > > On Mon, Aug 28, 2023 at 11:49 PM Conor Dooley <conor@kernel.org> wrote: > > >> > > >> On Mon, Aug 28, 2023 at 08:38:32PM +0800, Binbin Zhou wrote: > > >>> Document the Power Management Unit system controller compatible for > > >>> Loongson-2K2000. > > >>> > > >>> This is a missing compatible, now we add it. > > >>> > > >>> Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> > > >>> --- > > >>> .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml | 1 + > > >>> 1 file changed, 1 insertion(+) > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml > > >>> index da2dcfeebf12..7473c5659929 100644 > > >>> --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml > > >>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml > > >>> @@ -15,6 +15,7 @@ properties: > > >>> - enum: > > >>> - loongson,ls2k0500-pmc > > >>> - loongson,ls2k1000-pmc > > >>> + - loongson,ls2k2000-pmc > > >> > > >> Same thing here as the driver patch, the pmc on this newly added SoC > > >> appears to use the same codepaths as the existing ones. Does it share a > > >> programming model & should there be a fallback compatible here? > > > > > > I noticed the guideline about fallback compatible: > > > > > > "DO use fallback compatibles when devices are the same as or a subset > > > of prior implementations." > > > > > > But in fact, ls2k0500/ls2k1000/ls2k2000 are independent, there is no subset. > > > > We do not consider here ls2k0500/ls2k1000/ls2k2000, but PMC in each of > > them. If they are independent, why would they use the same interface? > > Sorry. I may have misunderstood. > The "subset" in the above guideline, here we should be talking about > PMC, not SoC. > For PMC, ls2k0500/ls2k1000/ls2k2000 are the same. > > Am I understanding correctly now? The test is can an OS/client which only understands the fallback compatible (loongson,ls2k-pmc) use that h/w block (or some subset of it)? If so, then a fallback is appropriate. If not, then don't use a fallback. If the block in one SoC is a proper subset of another SoC, then make the compatible that's a subset the fallback. However, that's really only helpful if an OS/client already understands the fallback. > Also, when I said "independent" above, I meant they are three different SoCs. But those SoCs are probably not designed completely independently. An existing SoC is the basis for the next SoC design. OTOH, things like pinmux, clocks, power mgt, etc. tend to change in every chip. Rob
diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml index da2dcfeebf12..7473c5659929 100644 --- a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml @@ -15,6 +15,7 @@ properties: - enum: - loongson,ls2k0500-pmc - loongson,ls2k1000-pmc + - loongson,ls2k2000-pmc - const: syscon reg:
Document the Power Management Unit system controller compatible for Loongson-2K2000. This is a missing compatible, now we add it. Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn> --- .../devicetree/bindings/soc/loongson/loongson,ls2k-pmc.yaml | 1 + 1 file changed, 1 insertion(+)