| 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
| ||
|
Date: Wed, 23 Apr 2014 09:38:10 -0700
From: Florian Fainelli <f.fainelli@...il.com>
To: Hubert Chaumette <hchaumette@...neo-embedded.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] ARM: i.MX6: Add OF configuration support for ksz9031
2014-04-23 0:35 GMT-07:00 Hubert Chaumette <hchaumette@...neo-embedded.com>:
> Le mardi 22 avril 2014 à 09:10 -0700, Florian Fainelli a écrit :
>> 2014-04-22 8:49 GMT-07:00 Hubert Chaumette <hchaumette@...neo-embedded.com>:
>> > Adds support for ksz9031 PAD skew configuration over devicetree.
>> >
>> > Changes since v1:
>> > - Removed ksz9021 and ksz9031 fixup deletions from arch/arm/mach-imx/mach-imx6q.c
>>
>> You should probably update the existing binding document in
>> Documentation/devicetree/bindings/net/micrel-ksz9021.txt to cover the
>> KSZ9031 PHY too.
>
> Should it be renamed in the process (micrel-ksz90x1.txt) ?
That's a good question, I suppose that you could proceed to a rename
after checking that this document is not referenced directly somewhere
else in Documentation/devicetree/bindings/
>
>> >
>> > Signed-off-by: Hubert Chaumette <hchaumette@...neo-embedded.com>
>> > ---
>> > drivers/net/phy/micrel.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++-
>> > 1 file changed, 184 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> > index 5a8993b..df56b26 100644
>> > --- a/drivers/net/phy/micrel.c
>> > +++ b/drivers/net/phy/micrel.c
>> > @@ -242,6 +242,189 @@ static int ksz9021_config_init(struct phy_device *phydev)
>> > return 0;
>> > }
>> >
>> > +#define MII_KSZ9031RN_MMD_CTRL_REG 0x0d
>> > +#define MII_KSZ9031RN_MMD_REGDATA_REG 0x0e
>> > +#define OP_DATA 1
>> > +#define KSZ9031_PS_TO_REG 60
>> > +
>> > +/* Extended registers */
>> > +#define MII_KSZ9031RN_CONTROL_PAD_SKEW 4
>> > +#define MII_KSZ9031RN_RX_DATA_PAD_SKEW 5
>> > +#define MII_KSZ9031RN_TX_DATA_PAD_SKEW 6
>> > +#define MII_KSZ9031RN_CLK_PAD_SKEW 8
>> > +
>> > +static int ksz9031_extended_write(struct phy_device *phydev,
>> > + u8 mode, u32 dev_addr, u32 regnum, u16 val)
>> > +{
>> > + phy_write(phydev, MII_KSZ9031RN_MMD_CTRL_REG, dev_addr);
>> > + phy_write(phydev, MII_KSZ9031RN_MMD_REGDATA_REG, regnum);
>> > + phy_write(phydev, MII_KSZ9031RN_MMD_CTRL_REG, (mode << 14) | dev_addr);
>> > + return phy_write(phydev, MII_KSZ9031RN_MMD_REGDATA_REG, val);
>> > +}
>> > +
>> > +static int ksz9031_extended_read(struct phy_device *phydev,
>> > + u8 mode, u32 dev_addr, u32 regnum)
>> > +{
>> > + phy_write(phydev, MII_KSZ9031RN_MMD_CTRL_REG, dev_addr);
>> > + phy_write(phydev, MII_KSZ9031RN_MMD_REGDATA_REG, regnum);
>> > + phy_write(phydev, MII_KSZ9031RN_MMD_CTRL_REG, (mode << 14) | dev_addr);
>> > + return phy_read(phydev, MII_KSZ9031RN_MMD_REGDATA_REG);
>> > +}
>> > +
>> > +/* Two 5-bit fields register */
>> > +static int ksz9031_load_clk_skew_values(struct phy_device *phydev,
>> > + struct device_node *of_node,
>> > + char *field1, char *field2)
>> > +{
>> > + int val1 = -1;
>> > + int val2 = -2;
>> > + int newval;
>> > + int matches = 0;
>> > +
>> > + if (!of_property_read_u32(of_node, field1, &val1))
>> > + matches++;
>> > +
>> > + if (!of_property_read_u32(of_node, field2, &val2))
>> > + matches++;
>> > +
>> > + if (!matches)
>> > + return 0;
>> > +
>> > + if (matches < 2)
>> > + newval = ksz9031_extended_read(phydev, OP_DATA, 2,
>> > + MII_KSZ9031RN_CLK_PAD_SKEW);
>> > + else
>> > + newval = 0;
>> > +
>> > + if (val1 != -1)
>> > + newval = (newval & 0xffe0) |
>> > + ((val1 / KSZ9031_PS_TO_REG) & 0x1f);
>> > +
>> > + if (val2 != -2)
>> > + newval = (newval & 0xfc1f) |
>> > + (((val2 / KSZ9031_PS_TO_REG) & 0x1f) << 5);
>> > +
>> > + return ksz9031_extended_write(phydev, OP_DATA, 2,
>> > + MII_KSZ9031RN_CLK_PAD_SKEW, newval);
>> > +}
>> > +
>> > +/* Four 4-bit fields register */
>> > +static int ksz9031_load_data_skew_values(struct phy_device *phydev,
>> > + struct device_node *of_node, u16 reg,
>> > + char *field1, char *field2,
>> > + char *field3, char *field4)
>> > +{
>> > + int val1 = -1;
>> > + int val2 = -2;
>> > + int val3 = -3;
>> > + int val4 = -4;
>> > + int newval;
>> > + int matches = 0;
>> > +
>> > + if (!of_property_read_u32(of_node, field1, &val1))
>> > + matches++;
>> > +
>> > + if (!of_property_read_u32(of_node, field2, &val2))
>> > + matches++;
>> > +
>> > + if (!of_property_read_u32(of_node, field3, &val3))
>> > + matches++;
>> > +
>> > + if (!of_property_read_u32(of_node, field4, &val4))
>> > + matches++;
>> > +
>> > + if (!matches)
>> > + return 0;
>> > +
>> > + if (matches < 4)
>> > + newval = ksz9031_extended_read(phydev, OP_DATA, 2, reg);
>> > + else
>> > + newval = 0;
>> > +
>> > + if (val1 != -1)
>> > + newval = (newval & 0xfff0) |
>> > + (((val1 / KSZ9031_PS_TO_REG) & 0xf) << 0);
>> > +
>> > + if (val2 != -2)
>> > + newval = (newval & 0xff0f) |
>> > + (((val2 / KSZ9031_PS_TO_REG) & 0xf) << 4);
>> > +
>> > + if (val3 != -3)
>> > + newval = (newval & 0xf0ff) |
>> > + (((val3 / KSZ9031_PS_TO_REG) & 0xf) << 8);
>> > +
>> > + if (val4 != -4)
>> > + newval = (newval & 0x0fff) |
>> > + (((val4 / KSZ9031_PS_TO_REG) & 0xf) << 12);
>>
>> How about something like this:
>>
>> for (i = 0; i < 4; i++) {
>> if (val[i] != -i) {
>> mask = 0xffff;
>> mask <<= (0 << (i * 4));
>> newval = (newval & mask) |
>> (((val[i] / KSZ9031_PS_TO_REG) & 0xf) << (i *4);
>> }
>> }
>>
>> Such that you could factor ksz9031_load_ctrl_skew_values() and
>> ksz9031_load_data_skew_values() to specify the number of matches they
>> should have?
>
> I'm on it.
>
> We could go further along this path and add a field length parameter to
> also factor ksz9031_load_clk_skew_values()?
Absolutely, that would be more future-proof and compact, so I am all
for it. Thanks!
> A similar loop could be used in ksz9021_load_values_from_of().
>
--
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
Powered by blists - more mailing lists