Porting guide ar71xx to ath79?

I see that there are 51 MikroTik routers that are supported by openwrt. I see there are 3 basic firmware releases: nand-64, nand-large, and rb-nor. The rest are the same just with WiFi drivers it looks like. What is a quick way to see which routers are supported by what firmware?

Darn it. I didn't scroll to the right so I didn't see the other fields.

Thank you.

I took a stab at this today, albeit in a separate file. Unfortunately, I still can't get my router to boot with this, but that probably has unrelated changes, so I can't really tell if this is working. @juppin, would you mind taking a look?
https://github.com/oxc/openwrt/commit/ce299694cf87d3dae49adbe47b867aef911c36b0

@musashino, perhaps you can test this on one your affected devices? I will have to wait until I get my usb-to-ttl converter so I can get a serial console on that router. Here is how I think this should work :slight_smile: https://github.com/oxc/openwrt/commit/wzr-hp-ag300h

Thank you for your work, @oxc ! (And, I forgot to reply to your mention, sorry...)

I tried your patch on my BHR-4GRV, but it fails to concatenate with following log:

[    0.597009] virt-mtdconcat virtual_flash: failed to get mtd device "virtual_flash"
[    0.616669] m25p80 spi0.0: w25q128 (16384 Kbytes)
[    0.633614] m25p80 spi0.1: w25q128 (16384 Kbytes)

dts:

Summary
/ {
	virtual_flash {
		compatible = "virtual,mtd-concat";
		devices = <&flash0 &flash1>;

		partitions {
			compatible = "fixed-partitions";
			#address-cells = <1>;
			#size-cells = <1>;

			partition@0 {
				label = "test";
				reg = <0x0000000 0x2000000>;
				read-only;
			};
		};
	};
};

&spi {
	status = "okay";
	num-cs = <2>;
	cs-gpios = <0>, <0>;

	flash0: flash@0 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "jedec,spi-nor";
		reg = <0>;
		spi-max-frequency = <25000000>;
	};

	flash1: flash@1 {
		#address-cells = <1>;
		#size-cells = <1>;
		compatible = "jedec,spi-nor";
		reg = <1>;
		spi-max-frequency = <25000000>;
	};
};

I am currently exploring the cause, perhaps the name of flash to be concatenated may be wrong.

I added printk( KERN_INFO "mtd part: %s\n", part) to see the flash name to be concatenated, and it says:

[    0.631794] mtd part: virtual_flash

It seems that it got own node name.

And, I added following changes:


Then, it succeeded to concatenate flash devices and detects mtd partitions!

log:

[    0.608590] m25p80 spi0.0: w25q128 (16384 Kbytes)
[    0.625545] m25p80 spi0.1: w25q128 (16384 Kbytes)
[    0.631734] mtd part: spi0.0
[    0.634655] mtd part: spi0.1
[    0.637550] Concatenating MTD devices:
[    0.641389] (0): "spi0.0"
[    0.644019] (1): "spi0.1"
[    0.646652] into device "virtual_flash"
[    0.650562] 1 fixed-partitions partitions found on MTD device virtual_flash
[    0.657563] Creating 1 MTD partitions on "virtual_flash":
[    0.663032] 0x000000000000-0x000002000000 : "test"

Great job!

@musashino, thanks, indeed I used the wrong node object. The fix is committed in https://github.com/oxc/openwrt/commit/49a6df312c338e82b54664263ca59d7f7cc92912, if you want to test again. The hardcoded spi names don't fit the generic setup of the driver, of course. I will finalize this when I get the chance to debug my own device.

Yes, of course.

Thank you for your fix, I tried again.
I got following error:

[    0.597132] virt-mtdconcat virtual_flash: failed to get mtd device "flash"
[    0.616123] m25p80 spi0.0: w25q128 (16384 Kbytes)
[    0.633098] m25p80 spi0.1: w25q128 (16384 Kbytes)

Thanks, I guess I'll have to find a better way to get the mtd device (or its name) from the OF node. I got this from another patch (605-rt2x00-load-eeprom-on-SoC-from-a-mtd-device-defines-.patch), but does not seem sufficient in our use case.

I had forgotten simple things.

If label is defined on the flash node, the name of flash device is replaced by it (ex.: spi0.0 -> flash0).

So, I added labels into flash node as following:

--- a/target/linux/ath79/dts/ar7242_buffalo_bhr-4grv.dts
+++ b/target/linux/ath79/dts/ar7242_buffalo_bhr-4grv.dts
@@ -87,6 +87,7 @@
                compatible = "jedec,spi-nor";
                reg = <0>;
                spi-max-frequency = <25000000>;
+               label = "flash0";
        };

        flash1: flash@1 {
@@ -95,6 +96,7 @@
                compatible = "jedec,spi-nor";
                reg = <1>;
                spi-max-frequency = <25000000>;
+               label = "flash1";
        };
 };

However, by this alone, the spi-nor flash is not probed at the timing when the mtd-concat driver tries to concatenate:

[    0.596914] virt-mtdconcat virtual_flash: failed to get mtd device "flash0"
[    0.615969] m25p80 spi0.0: w25q128 (16384 Kbytes)
[    0.632943] m25p80 spi0.1: w25q128 (16384 Kbytes)

I also made the following changes:


Then, mtd-concat driver succeeded to concatenate flash devices!

[    0.607397] m25p80 spi0.0: w25q128 (16384 Kbytes)
[    0.624417] m25p80 spi0.1: w25q128 (16384 Kbytes)
[    0.630606] Concatenating MTD devices:
[    0.634388] (0): "flash0"
[    0.637020] (1): "flash1"
[    0.639725] into device "virtual_flash"
[    0.643617] 1 fixed-partitions partitions found on MTD device virtual_flash
[    0.650633] Creating 1 MTD partitions on "virtual_flash":
[    0.656079] 0x000000000000-0x000002000000 : "test"


I also implemented generic concatinating driver and added 'num-cs' DT property into ath79 SPI driver.



In my own implementation, specifies concatinated child MTDs by DT node handle (phandle).
So, I also added get_mtd_device_by_node() in mtdcore.c

This patch is not needed and will be never accepted because you can increase the number of supported cs gpios with cs-gpios in a generic way independent of the spi driver.
For reference take a look at line 34 in spi-bus binding doc.
To use all three hw cs gpios you define simply (no need for patching driver):

num-cs = 3;
cs-gpios = <0>, <0>, <0>;

Note: The setup of the hw cs gpios should be setup from the bootloader or with a pinmux!

In comparison to @oxc implementation the implementation of @drvlabo looks a bit ugly.

@oxc Your implememtation looks good to me and clean to be accepted if you figure out why it´s only working in the main mtd driver dir.

Thanks! I believe the solution to this is deferred probing, which I have already implemented, and will test later today when my USB-to-TTL cables arrive (in the meantime I figured out I could have also used one of the arduinos lying around for that task :smiley: )

In the meantime I could really benefit from @drvlabo's 463-mtd-get-device-by-DT-node.patch patch.
Does this change make sense? Is it likely to get accepted (on its own, maybe)? The implementation looks straight enough to me (it's basically a copy of get_mtd_device_nm).

Yeah that was working good for me too in the past :slight_smile:

I think the main problem with your implementation is the top down probe order in the device tree...
The virtual concat device is probed before the real mtd devices are available and therefore no mtd name is matching.
The get_mtd_device_by_node function will solve this, but i don´t know if this one will be accepted. Probably yes.

You could also try to move your virtual concat node into the spi node after the two spi nor nodes to get the real devices probed first.
For me this wouldn´t look that bad in the spi node, because it is related to the two spi nor flash devices.
What do you think about the node placement in the spi node after the real devices?

I don't think so, the devices would still not be registered, so the function would not find any device with that of_node set.

Still, loading mtd devices by of_node feels cleaner than trying label and devname, especially since I haven't figured out where the "spi0.0“ names in @musashino's example come from and how to generate them.

Re-ordering the device tree would definitely be my preferred solution, that should be used even if deferred probing is implemented (to prevent unnecessary deferring). I will try that this afternoon.

The name comes from the order of registered spi devices and the cs num...
The setup begins in drivers/spi/spi.c and the name is set here in line 483.

If you do not set a label in dt, the label becomes the name of the device. So therefore the first spi slave on the first spi device is labled as spi0.0, the second slave is spi0.1 and so on...
This shouldn matter and i think your implementation should also work if you don´t set the label expicit for a spi slave device.
But you have to test it on real hw. :slight_smile:

I think you are right.

Probably, but why add code that is not forcedly needed.

When enabling mdio0 in DT file, kernel panics in ag71xx_mdio_reset(), because of unaligned access.

Then, we need the below patch.

diff -Nurp a/drivers/net/ethernet/atheros/ag71xx/ag71xx_mdio.c b/drivers/net/ethernet/atheros/ag71xx/ag71xx_mdio.c
--- a/drivers/net/ethernet/atheros/ag71xx/ag71xx_mdio.c 2018-08-22 07:52:41.769806773 +0900
+++ b/drivers/net/ethernet/atheros/ag71xx/ag71xx_mdio.c 2018-08-27 21:19:46.042578603 +0900
@@ -183,8 +183,8 @@ static int ag71xx_mdio_probe(struct plat
                return -ENOMEM;
 
        am->mii_regmap = syscon_regmap_lookup_by_phandle(np, "regmap");
-       if (!am->mii_regmap)
-               return -ENOENT;
+       if (IS_ERR(am->mii_regmap))
+               return PTR_ERR(am->mii_regmap);
 
        mii_bus = devm_mdiobus_alloc(amdev);
        if (!mii_bus)

After applying this patch, ag71xx-mdio fails with -22, that is, syscon_regmap_lookup_by_phandle() return -ENODEV.

This looks indeed like a bug, the function returns an ERR_PTR, and your fix looks correct. You should submit that patch either way :slight_smile:

ag71xx_mdio_probe() must return zero or error code, not a pointer value. So, my patch is corrent.

i'm pretty sure i ran into this same issue when porting the routerstation pro to ath79.

i was able to work around it by adding "syscon" to the compatible line like so:

&eth0 {
	compatible = "qca,ar7100-eth", "syscon";
	phy-mode = "rgmii";
	phy-handle = <&phy4>;
};

if this ends up being merged into master, perhaps https://github.com/openwrt/openwrt/blob/master/target/linux/ath79/dts/ar7161_ubnt_routerstation-pro.dts could be modified at the same time.