On Saturday 09 October 2021 12:09:28 Robert Marko wrote:
> On Sat, Oct 9, 2021 at 11:42 AM Marek Behún <ka...@kernel.org> wrote:
> >
> > > > > > +config MVEBU_MAC_HW_INFO_OFFSET
> > > > > > + hex "Marvell hw_info (mac) SPI flash offset"
> > > > > > + depends on MVEBU_MAC_HW_INFO
> > > > > > + default 0x3E0000
> > > > > > + help
> > > > > > + This option defines the SPI flash offset of the Marvell
> > > > > > + hw_info area. This defaults to 0x3E0000 on most Armada
> > > > > > + A3720 platforms.
> > > > >
> > > > > Have you tried to specify this offset directly into DTS file? Because
> > > > > in DTS file is already specified this hw info partition and it seems
> > > > > like that this kind of information belongs to DTS.
> > > >
> > > > I haven't encountered a board, which has a different offset so far.
> > > > This can be treated as a nicer way of defining this offset, rather
> > > > than just hard-coding it in the source code itself.
> > > >
> > > > In case there are more boards with this partition, a way of defining
> > > > the offset in the DTS can be added later and this value can then
> > > > be used as a default.
> > >
> > > +Marek
> > >
> > > My understanding is that all these definitions, like memory address
> > > spaces, partitions, etc... belong to DTS file (or plat structures for
> > > boards which do not use DT) and not into source code or config options
> > > as they are describing hw layout.
> > >
> > > There is ongoing process to move / convert SPI partitions from source
> > > files and config defines into DTS files, so for me this looks like a
> > > step backward...
> > >
> > > But I would like to hear opinion also from other people.
> >
> > This should be done in device tree. Device tree has bindings for such
> > things. You should be able to do something like this:
> >
> > spi-flash@1 {
> > partitions {
> > compatible = "fixed-partitions";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > board_info: board-info@3E0000 {
> > compatible = "nvmem-cells";
> > label = "board-info";
> > reg = <0x3E0000 0x1000>;
> > read-only;
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > mac_addr_eth0: mac-addr1@0 {
> > reg = <0x0 0x6>;
> > };
> >
> > mac_addr_eth1: mac-addr2@1 {
> > reg = <0x6 0x6>;
> > };
> > };
> > };
> > };
> >
> > ethernet@30000 {
> > nvmem-cells = <&mac_addr_eth0>;
> > nvmem-cell-names = "mac-address";
> > };
>
> The thing is that the position of each value is not fixed, they can be
> in whatever order or position inside of the partition and they are not
> TLV
> as there is no length argument stored.
> They are really similar to the traditional U-boot env so you cant use
> generic NVMEM cells and parse by position and length.
> But I am sure that we can add a compatible for the hw_info parser and
> then it can parse the reg property directly instead of using KConfig
> for that.
Something like this? compatible = "marvell,hw-info"
> I am sure Luka knows more about the format than me.
>
> Regards,
> Robert
>
> >
> > Look at
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
> >
> > Also we need to add nvmem API into U-Boot and get rid of the ad-hoc
> > efuse/mac/hw_mac nonsense.
>
> This would be great for a lot of devices.
> >
> > Marek
>
>
>
> --
> Robert Marko
> Staff Embedded Linux Engineer
> Sartura Ltd.
> Lendavska ulica 16a
> 10000 Zagreb, Croatia
> Email: robert.ma...@sartura.hr
> Web: www.sartura.hr
