| Message ID | cover.1693218539.git.zhoubinbin@loongson.cn |
|---|---|
| Headers | show |
| Series | soc: loongson: Fix some issues about loongson_pm2 | expand |
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
Hi all: Since commit 67694c076bd7 ("soc: loongson2_pm: add power management support"), the Loongson-2K PM driver was added, some issues have been found and this patch set is planned to fix these issues. Specific: Patch 1: Compilation error found by Randy; Patch 2/3: Add Loongson-2K2000 support; Patch 4/5: Add Loongson-2K SoC reboot/shutdown support as part of power management. Thanks. Binbin Zhou (5): soc: loongson: loongson_pm2: add dependency for INPUT dt-bindings: soc: loongson,ls2k-pmc: Add missing compatible for Loongson-2K2000 soc: loongson: loongson_pm2: Add missing compatible for Loongson-2K2000 dt-bindings: soc: loongson,ls2k-pmc: Allow syscon-reboot/syscon-poweroff as child soc: loongson: loongson_pm2: Populate children syscon nodes .../soc/loongson/loongson,ls2k-pmc.yaml | 30 ++++++++++++++++++- drivers/soc/loongson/Kconfig | 1 + drivers/soc/loongson/loongson2_pm.c | 7 +++++ 3 files changed, 37 insertions(+), 1 deletion(-)