linux-kernel - Re: [PATCH v5 2/4] devicetree: bindings: Document Krait CPU/L1 EDAC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 7 Mar 2014 15:08:56 -0800
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc:	Kumar Gala <galak@...eaurora.org>, Borislav Petkov <bp@...en8.de>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	Mark Rutland <Mark.Rutland@....com>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH v5 2/4] devicetree: bindings: Document Krait CPU/L1 EDAC

On 02/26, Lorenzo Pieralisi wrote:
> On Tue, Feb 25, 2014 at 08:48:38PM +0000, Kumar Gala wrote:
> > 
> > On Feb 25, 2014, at 5:16 AM, Lorenzo Pieralisi <lorenzo.pieralisi@....com> wrote:
> > > 
> > > As I mentioned, I do not like the idea of adding compatible properties
> > > just to force the kernel to create platform devices out of device tree
> > > nodes. On top of that I would avoid adding a compatible property
> > > to the cpus node (after all properties like enable-method are common for all
> > > cpus but still duplicated), my only concern being backward compatibility
> > > here (ie if we do that for interrupts, we should do that also for other
> > > common cpu nodes properties, otherwise we have different rules for
> > > different properties).
> > > 
> > > I think you can then add interrupts to cpu nodes ("qcom,krait" specific),
> > > and as you mentioned create a platform device for that.
> > > 
> > > Thanks,
> > > Lorenzo
> > 
> > So I agree with the statement about adding compatibles just to create platform devices is wrong.  However its seems perfectly reasonable for a cpu node to have a compatible property.  I don't see why a CPU is any different from any other device described in a DT.
> 
> I was referring to the /cpus node, not to individual cpu nodes, where
> the compatible property is already present now.
> 

Ok I think I'll go ahead with moving the interrupts into each cpu node, i.e.:

        cpus {  
                #address-cells = <1>;
                #size-cells = <0>;

                cpu@0 { 
                        compatible = "qcom,krait";
                        device_type = "cpu";
                        reg = <0>;
                        interrupts = <1 14 0x304>;
                        next-level-cache = <&L2>;
                };

                cpu@1 { 
                        compatible = "qcom,krait";
                        device_type = "cpu";
                        reg = <1>;
                        interrupts = <1 14 0x304>;
                        next-level-cache = <&L2>;
                };

                L2: l2-cache {
                        compatible = "cache";
                        interrupts = <0 2 0x4>;
		};
	};

Or should we be expressing the L1 cache as well? Something like:

        cpus {  
                #address-cells = <1>;
                #size-cells = <0>;

                cpu@0 { 
                        compatible = "qcom,krait";
                        device_type = "cpu";
                        reg = <0>;
                        next-level-cache = <&L1_0>;

			L1_0: l1-cache {
				compatible = "arm,arch-cache";
				interrupts = <1 14 0x304>;
				next-level-cache = <&L2>;
			}
                };

                cpu@1 { 
                        compatible = "qcom,krait";
                        device_type = "cpu";
                        reg = <1>;
                        next-level-cache = <&L1_1>;

			L1_1: l1-cache {
				compatible = "arm,arch-cache";
				interrupts = <1 14 0x304>;
				next-level-cache = <&L2>;
			}
                };

                L2: l2-cache {
                        compatible = "arm,arch-cache";
                        interrupts = <0 2 0x4>;
		};
	};

(I'm also wondering if the 3rd cell of the interrupt binding
should only indicate the CPU that the interrupt property is
inside?)

Finally we can have the edac driver look for a "qcom,krait"
compatible node in cpus that it can create a platform device for,
i.e..

static int __init krait_edac_driver_init(void)
{
        struct device_node *np;

        np = of_get_cpu_node(0, NULL);
        if (!np)
                return 0;

        if (!krait_edacp && of_device_is_compatible(np, "qcom,krait"))
                krait_edacp = of_platform_device_create(np, "krait_edac", NULL);
        of_node_put(np);

        return platform_driver_register(&krait_edac_driver);
}
module_init(krait_edac_driver_init);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/