linux-kernel - Re: [PATCH] net: Add MOXA ART SoCs ethernet driver

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CACmBeS0Q_WNc17VSALyMPPKzN1wyLUccBLDJpbapVwxq4gdytQ@mail.gmail.com>
Date:	Fri, 26 Jul 2013 15:12:01 +0200
From:	Jonas Jensen <jonas.jensen@...il.com>
To:	Florian Fainelli <florian@...nwrt.org>
Cc:	netdev <netdev@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>, linux-doc@...r.kernel.org,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	arm@...nel.org
Subject: Re: [PATCH] net: Add MOXA ART SoCs ethernet driver

Hi Florian,

Thanks for your help on IRC with this. I think most of the issues you
pointed out should be fixed. Comments on still unresolved below:

On 22 July 2013 13:09, Florian Fainelli <florian@...nwrt.org> wrote:
>> +++ b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
>> @@ -0,0 +1,25 @@
>> +MOXA ART Ethernet Controller
>> +
>> +Required properties:
>> +
>> +- compatible : Should be "moxa,moxart-mac"
>> +- reg : Should contain registers location and length
>> +  index 0 : main register
>> +  index 1 : mac address (stored on flash)
>
> That is kind of unusual, since the MAC register is within the range of
> the MAC base register, just read the MAC from the base register and
> use that to fill in dev->dev_addr in your ndo_open() callback.

It is, but MAC is zeroed out in controller register after boot. That's why it
has to be read from flash memory (bootloader section), and the reason
it needs to map flash memory at all.

>> +
>> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> +
>> +       /* the flash driver (physmap_of) requests the same region
>> +        * so use ioremap instead of devm_ioremap_resource
>> +        */
>> +       priv->flash_base = ioremap(res->start, resource_size(res));
>> +       if (IS_ERR(priv->flash_base)) {
>> +               dev_err(p_dev, "%s: devm_ioremap_resource res_flash failed\n",
>> +                       __func__);
>> +               goto init_fail;
>> +       }
>
> As stated above from the Device Tree binding documentation, this is
> unnecessary, since you have a get_mac() function.

This is what the node looks like right now:

mac0: mac@...00000 {
compatible = "moxa,moxart-mac";
reg = <0x90900000 0x100>,
        <0x80000050 0x6>;
interrupts = <25 0>;
};

Bootloader starts at 0x80000000, looks like MAC is stored in code. I
currently see no way around mapping both registers.

Best regards,
Jonas
--
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/