| 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
| ||
|
Message-ID:
<IA0PR12MB7699ED80FAAFD3CF0B4F5502DC212@IA0PR12MB7699.namprd12.prod.outlook.com>
Date: Wed, 20 Nov 2024 10:57:55 +0000
From: "Mahapatra, Amit Kumar" <amit.kumar-mahapatra@....com>
To: Miquel Raynal <miquel.raynal@...tlin.com>
CC: "tudor.ambarus@...aro.org" <tudor.ambarus@...aro.org>, "michael@...le.cc"
<michael@...le.cc>, "broonie@...nel.org" <broonie@...nel.org>,
"pratyush@...nel.org" <pratyush@...nel.org>, "richard@....at"
<richard@....at>, "vigneshr@...com" <vigneshr@...com>, "robh@...nel.org"
<robh@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "Abbarapu, Venkatesh"
<venkatesh.abbarapu@....com>, "linux-spi@...r.kernel.org"
<linux-spi@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-mtd@...ts.infradead.org"
<linux-mtd@...ts.infradead.org>, "nicolas.ferre@...rochip.com"
<nicolas.ferre@...rochip.com>, "alexandre.belloni@...tlin.com"
<alexandre.belloni@...tlin.com>, "claudiu.beznea@...on.dev"
<claudiu.beznea@...on.dev>, "Simek, Michal" <michal.simek@....com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "alsa-devel@...a-project.org"
<alsa-devel@...a-project.org>, "patches@...nsource.cirrus.com"
<patches@...nsource.cirrus.com>, "git (AMD-Xilinx)" <git@....com>,
"amitrkcian2002@...il.com" <amitrkcian2002@...il.com>, "beanhuo@...ron.com"
<beanhuo@...ron.com>
Subject: RE: [RFC PATCH 2/2] dt-bindings: spi: Update stacked and parallel
bindings
> > spi@0 {
> > ...
> > flash@0 {
> > compatible = "jedec,spi-nor"
> > reg = <0x00>;
> > stacked-memories = <&flash@0 &flash@1>;
> > spi-max-frequency = <50000000>;
> > ...
> > partitions {
> > compatible = "fixed-partitions";
> > concat-partition = <&flash0_part0 &flash1_part0>;
> >
> > flash0_part0: partition@0 {
> > label = "part0_0";
> > reg = <0x0 0x800000>;
> > };
> > };
> > };
> > flash@1 {
> > compatible = "jedec,spi-nor"
> > reg = <0x01>;
> > stacked-memories = <&flash@0 &flash@1>;
> > spi-max-frequency = <50000000>;
> > ...
> > partitions {
> > compatible = "fixed-partitions";
> > concat-partition = <&flash0_part0 &flash1_part0>;
> >
> > flash1_part0: partition@0 {
> > label = "part0_1";
> > reg = <0x0 0x800000>;
> > };
> > };
> > };
> > };
> >
> >>
> >> > compatible = "fixed-partitions";
> >> > concat-partition = <&flash0_partition &flash1_partition>;
> >> > flash1_partition: partition@0 {
> >> > label = "part0_1";
> >> > reg = <0x0 0x800000>;
> >> > }
> >> > }
> >> > }
> >> >
> >> > }
> >> >
> >> > parallel-memories binding changes:
> >> > - Remove the size information from the bindings and change the type to
> >> > boolen.
> >> > - Each flash connected in parallel mode should be identical and will have
> >> > one flash node for both the flash devices.
> >> > - The “reg” prop will contain the physical CS number for both the connected
> >> > flashes.
> >> >
> >> > The new layer will double the mtd-> size and register it with the
> >> > mtd layer.
> >>
> >> Not so sure about that, you'll need a new mtd device to capture the whole
> device.
> >> But this is implementation related, not relevant for binding.
> >>
> >> >
> >> > spi@1 {
> >> > ...
> >> > flash@3 {
> >> > compatible = "jedec,spi-nor"
> >> > reg = <0x00 0x01>;
> >> > paralle-memories ;
> >>
> >> Please fix the typos and the spacing (same above).
> >>
> >> > spi-max-frequency = <50000000>;
> >> > ...
> >> > partitions {
> >> > compatible = "fixed-partitions";
> >> > flash0_partition: partition@0 {
> >> > label = "part0_0";
> >> > reg = <0x0 0x800000>;
> >> > }
> >> > }
> >> > }
> >> > }
> >> >
> >> > Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>
> >> > ---
> >> > .../bindings/spi/spi-controller.yaml | 23 +++++++++++++++++--
> >> > .../bindings/spi/spi-peripheral-props.yaml | 9 +++-----
> >> > 2 files changed, 24 insertions(+), 8 deletions(-)
> >> >
> >> > diff --git
> >> > a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> >> > b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> >> > index 093150c0cb87..2d300f98dd72 100644
> >> > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml
> >> > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml
> >> > @@ -185,7 +185,26 @@ examples:
> >> > flash@2 {
> >> > compatible = "jedec,spi-nor";
> >> > spi-max-frequency = <50000000>;
> >> > - reg = <2>, <3>;
> >> > - stacked-memories = /bits/ 64 <0x10000000 0x10000000>;
> >> > + reg = <2>;
> >> > + stacked-memories = <&flash0 &flash1>;
> >> > };
> >>
> >> I'm sorry but this is not what you've talked about in this series.
> >> Either you have flash0 and flash1 and use the stacked-memories
> >> property in both of them (which is what you described) or you create
> >> a third virtual device which points to two other flashes. This
> >> example allows for an easier use of the partitions
> >
> > If I understand your point correctly, you're suggesting that we should
> > avoid using stacked-memories and concat-partition properties together
> > and instead choose one approach. Between the two, I believe
> > concat-partition would be the better option.
>
> That's not exactly it, look at the reg properties above, they do not match the flash
> devices. Your example above invalid but it is not clear whether this is another typo
> or voluntary.
Ah, I see. It's a typo ☹. However, based on the discussion below, it seems
we might not need the 'stacked-memories' property anymore.
>
> > While looking into your mtdconcat patch [1], I noticed that it creates
> > a virtual MTD device that points to partitions on two different flash
> > nodes, which aligns perfectly with our requirements.
> >
> > However, there are two key concerns that, if addressed, could make
> > this patch suitable for the stacked mode:
> >
> > 1/ The creation of a virtual device that does not have a physical
> > existence.
>
> We do already have:
> - the master mtd device (disabled by default for historical reasons, but
> can be enabled with a Kconfig option).
> - an mtd device per partition
>
> I don't see a problem in creating virtual mtd devices in the kernel.
>
> > 2/ The creation of individual MTD devices that are concatenated to
> > form the virtual MTD device, which may not be needed by the user.
>
> You can also get rid of them by default (or perhaps do the opposite and let a Kconfig
> option for that).
Ok, great! I'll look into it and try to integrate it into your
previous patch [1]. If everything works out, then IMHO this(patch[1] + the
above changes) should suffice for adding stacked support.
Regards,
Amit
>
> > Regarding the first point, I currently cannot think of a better
> > generic way to support the stacked feature than creating a virtual device.
> > Please let me know you thoughts on this.
> >
> > For the second point, one possible solution is to hide the individual
> > MTD devices (that form the concatenated virtual MTD device) from the
> > user once the virtual device is created. Please let us know if you
> > have any other suggestions to address this issue.
>
> That is what is done with the master device by default.
>
> > [1]
> > https://lore.kernel.org/linux-mtd/20191127105522.31445-5-miquel.raynal
> > @bootlin.com/
>
> Thanks,
> Miquèl
Powered by blists - more mailing lists