netdev - Re: [PATCH 2/2] ethernet: arc: Add support for Rockchip SoC glue layer device tree bindings

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: <CAGVrzcbBNnwD9UHgPweDMCtDJYPdm=j5Gn53-qf2w462hYLodQ@mail.gmail.com>
Date:	Fri, 8 Aug 2014 20:21:59 -0700
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Beniamino Galvani <b.galvani@...il.com>
Cc:	Romain Perier <romain.perier@...il.com>,
	David Miller <davem@...emloft.net>, heiko@...ech.de,
	max.schwarz@...ine.de, Eric Dumazet <eric.dumazet@...il.com>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH 2/2] ethernet: arc: Add support for Rockchip SoC glue
 layer device tree bindings

2014-08-08 13:28 GMT-07:00 Beniamino Galvani <b.galvani@...il.com>:
> On Fri, Aug 08, 2014 at 12:27:55PM +0000, Romain Perier wrote:
>> This patch defines a platform glue layer for Rockchip SoCs which support
>> arc-emac driver. It ensures that regulator for the rmii is on before trying
>> to connect to the ethernet controller. It applies right speed and mode changes
>> to the grf when ethernet settings changes.
>>
>> Signed-off-by: Romain Perier <romain.perier@...il.com>
>
> (...)
>
>> +struct rockchip_priv_data {
>> +     struct regmap *grf;
>> +     unsigned int grf_offset;
>> +     int interface;
>> +     struct regulator *regulator;
>> +};
>> +
>> +static const unsigned int rockchip_emac_soc_grf_offset[] = {
>> +     0x154, /* rk3066 */
>> +     0x0a4, /* rk3188 */
>> +};
>> +
>> +static const struct of_device_id rockchip_emac_dt_ids[] = {
>> +     { .compatible = "rockchip,rk3066-emac", .data = (void *)rockchip_emac_soc_grf_offset },
>> +     { .compatible = "rockchip,rk3188-emac", .data = (void *)rockchip_emac_soc_grf_offset + 1 },
>
> Hi Romain,
>
> I think you need parenthesis around "rockchip_emac_soc_grf_offset + 1"
> because the cast has higher precedence. Anyway, wouldn't it be simpler
> to avoid the indirection and pass the offset directly in the data field?

I would create separate structures for rk3066 and rk3188, it might
just contain a single member for now which is an offset, but in case
you need to extend it with more parameters, it's much easier, and you
won't have to rework the matching logic. The current use of +1 is
extremely error prone.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html