| Message ID | 1348195976-31703-1-git-send-email-marex@denx.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Marek Vasut <marex@denx.de> Date: Fri, 21 Sep 2012 04:52:54 +0200 > + phy_write(phydev, MII_KSZPHY_OMSO, > + KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE); This is not indented properly. The goal is not to exclusively use TAB characters to indent code until it sort-of looks fine. Rather, the goal is to properly line up function arguments with the first column after the openning parenthesis on the previous line. Using TAB and SPACE characters, as needed. > + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause > + | SUPPORTED_Asym_Pause), This is similarly not styled properly. Besies being indented imporperly on the second line, the final "|" character should be at the end of the first line, rather than start the second line. Resubmit this entire patch series, not just this one patch, once you've made these corrections. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear David Miller, > From: Marek Vasut <marex@denx.de> > Date: Fri, 21 Sep 2012 04:52:54 +0200 > > > + phy_write(phydev, MII_KSZPHY_OMSO, > > + KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE); > > This is not indented properly. The goal is not to exclusively use > TAB characters to indent code until it sort-of looks fine. The goal was to avoid checkpatch trouble. > Rather, the goal is to properly line up function arguments with > the first column after the openning parenthesis on the previous > line. Using TAB and SPACE characters, as needed. > > > + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause > > + | SUPPORTED_Asym_Pause), > > This is similarly not styled properly. This is copy-pasted from other entry. I'd hate to reformat the whole file. > Besies being indented imporperly on the second line, the final "|" > character should be at the end of the first line, rather than > start the second line. DTTO here. > Resubmit this entire patch series, not just this one patch, once > you've made these corrections. > > Thanks. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Marek Vasut <marex@denx.de> Date: Fri, 21 Sep 2012 20:04:09 +0200 >> Rather, the goal is to properly line up function arguments with >> the first column after the openning parenthesis on the previous >> line. Using TAB and SPACE characters, as needed. >> >> > + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause >> > + | SUPPORTED_Asym_Pause), >> >> This is similarly not styled properly. > > This is copy-pasted from other entry. I'd hate to reformat the whole file. I'm not asking you to, just the new code you're adding. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear David Miller, > From: Marek Vasut <marex@denx.de> > Date: Fri, 21 Sep 2012 20:04:09 +0200 > > >> Rather, the goal is to properly line up function arguments with > >> the first column after the openning parenthesis on the previous > >> line. Using TAB and SPACE characters, as needed. > >> > >> > + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause > >> > + | SUPPORTED_Asym_Pause), > >> > >> This is similarly not styled properly. > > > > This is copy-pasted from other entry. I'd hate to reformat the whole > > file. > > I'm not asking you to, just the new code you're adding. I don't like it. What'd you say to me linting the whole file with checkpatch in a subsequent patch? Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Marek Vasut <marex@denx.de> Date: Fri, 21 Sep 2012 20:33:12 +0200 > I don't like it. Then I'm not applying your patches. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear David Miller, > From: Marek Vasut <marex@denx.de> > Date: Fri, 21 Sep 2012 20:33:12 +0200 > > > I don't like it. > > Then I'm not applying your patches. What about the other part of my offer ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Marek Vasut <marex@denx.de> Date: Fri, 21 Sep 2012 20:50:46 +0200 > Dear David Miller, > >> From: Marek Vasut <marex@denx.de> >> Date: Fri, 21 Sep 2012 20:33:12 +0200 >> >> > I don't like it. >> >> Then I'm not applying your patches. > > What about the other part of my offer ? I read it, and if it were relevant to this discussion I would have quoted it. Can you please just fix up your patches exactly how I have asked you to so that I can get these bug fixes into the 'net' tree? This is completely rediculous, nobody else pushes back when I make simple coding style correction requests for their changes like you are. You're behavior is self defeating, it's causing your work to not be useful and to not propagate to the very place where people can benefit from it the most. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear David Miller, > From: Marek Vasut <marex@denx.de> > Date: Fri, 21 Sep 2012 20:50:46 +0200 > > > Dear David Miller, > > > >> From: Marek Vasut <marex@denx.de> > >> Date: Fri, 21 Sep 2012 20:33:12 +0200 > >> > >> > I don't like it. > >> > >> Then I'm not applying your patches. > > > > What about the other part of my offer ? > > I read it, and if it were relevant to this discussion I would > have quoted it. > > Can you please just fix up your patches exactly how I have asked you > to so that I can get these bug fixes into the 'net' tree? Sure, as you wish. > This is completely rediculous, nobody else pushes back when I make > simple coding style correction requests for their changes like you > are. You know, youth and all ... I was under the impression the patches shall be checkpatch clean. But you got me there quite well, something must be wrong with my precommit hook. > You're behavior is self defeating, it's causing your work to not be > useful and to not propagate to the very place where people can benefit > from it the most. Hey, this hurt. Anyway, about the checkpatch cleanup of the file, will that be welcome (afterwards I fix the patchset and repost)? Now that you pointed out there's more trouble in the file that is. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Marek Vasut <marex@denx.de> Date: Fri, 21 Sep 2012 21:06:52 +0200 > You know, youth and all ... I was under the impression the patches shall be > checkpatch clean. But you got me there quite well, something must be wrong with > my precommit hook. checkpatch is not a panacea, and it is in particular not an automaton that one uses without using any human judgement at all. In particular, checkpatch does not enforce the comment style we use in the networking code nor several other conventions that we use which are slightly different from the rest of the tree. Therefore strick checkpatch conformance is never appropriate. > Anyway, about the checkpatch cleanup of the file, will that be > welcome (afterwards I fix the patchset and repost)? See above, strict checkpatch cleanups, especially those done in a completely automaton style with zero human judgment involved, are not welcome. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Dear David Miller, > From: Marek Vasut <marex@denx.de> > Date: Fri, 21 Sep 2012 21:06:52 +0200 > > > You know, youth and all ... I was under the impression the patches shall > > be checkpatch clean. But you got me there quite well, something must be > > wrong with my precommit hook. > > checkpatch is not a panacea, and it is in particular not an automaton > that one uses without using any human judgement at all. > > In particular, checkpatch does not enforce the comment style we use in > the networking code nor several other conventions that we use which > are slightly different from the rest of the tree. > > Therefore strick checkpatch conformance is never appropriate. Understood. > > Anyway, about the checkpatch cleanup of the file, will that be > > welcome (afterwards I fix the patchset and repost)? > > See above, strict checkpatch cleanups, especially those done in > a completely automaton style with zero human judgment involved, > are not welcome. I meant the .features field ... it seems that the | at the following line appears in other PHY drivers as well though. Lets leave it at that anyway. Best regards, Marek Vasut -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c index cf287e0..b5723b9 100644 --- a/drivers/net/phy/micrel.c +++ b/drivers/net/phy/micrel.c @@ -21,6 +21,12 @@ #include <linux/phy.h> #include <linux/micrel_phy.h> +/* Operation Mode Strap Override */ +#define MII_KSZPHY_OMSO 0x16 +#define KSZPHY_OMSO_B_CAST_OFF (1 << 9) +#define KSZPHY_OMSO_RMII_OVERRIDE (1 << 1) +#define KSZPHY_OMSO_MII_OVERRIDE (1 << 0) + /* general Interrupt control/status reg in vendor specific block. */ #define MII_KSZPHY_INTCS 0x1B #define KSZPHY_INTCS_JABBER (1 << 15) @@ -101,6 +107,13 @@ static int kszphy_config_init(struct phy_device *phydev) return 0; } +static int ksz8021_config_init(struct phy_device *phydev) +{ + phy_write(phydev, MII_KSZPHY_OMSO, + KSZPHY_OMSO_B_CAST_OFF | KSZPHY_OMSO_RMII_OVERRIDE); + return 0; +} + static int ks8051_config_init(struct phy_device *phydev) { int regval; @@ -128,6 +141,19 @@ static struct phy_driver ksphy_driver[] = { .config_intr = ks8737_config_intr, .driver = { .owner = THIS_MODULE,}, }, { + .phy_id = PHY_ID_KSZ8021, + .phy_id_mask = 0x00ffffff, + .name = "Micrel KSZ8021", + .features = (PHY_BASIC_FEATURES | SUPPORTED_Pause + | SUPPORTED_Asym_Pause), + .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT, + .config_init = ksz8021_config_init, + .config_aneg = genphy_config_aneg, + .read_status = genphy_read_status, + .ack_interrupt = kszphy_ack_interrupt, + .config_intr = kszphy_config_intr, + .driver = { .owner = THIS_MODULE,}, +}, { .phy_id = PHY_ID_KS8041, .phy_id_mask = 0x00fffff0, .name = "Micrel KS8041", @@ -203,6 +229,7 @@ static struct mdio_device_id __maybe_unused micrel_tbl[] = { { PHY_ID_KSZ9021, 0x000ffffe }, { PHY_ID_KS8001, 0x00ffffff }, { PHY_ID_KS8737, 0x00fffff0 }, + { PHY_ID_KSZ8021, 0x00ffffff }, { PHY_ID_KS8041, 0x00fffff0 }, { PHY_ID_KS8051, 0x00fffff0 }, { } diff --git a/include/linux/micrel_phy.h b/include/linux/micrel_phy.h index 61f0905..be7f366 100644 --- a/include/linux/micrel_phy.h +++ b/include/linux/micrel_phy.h @@ -5,6 +5,7 @@ #define PHY_ID_KSZ9021 0x00221610 #define PHY_ID_KS8737 0x00221720 +#define PHY_ID_KSZ8021 0x00221555 #define PHY_ID_KS8041 0x00221510 #define PHY_ID_KS8051 0x00221550 /* both for ks8001 Rev. A/B, and for ks8721 Rev 3. */
The KSZ8021 PHY was previously caught by KS8051, which is not correct. This PHY needs additional setup if it is strapped for address 0. In such case an reserved bit must be written in the 0x16, "Operation Mode Strap Override" register. According to the KS8051 datasheet, that bit means "PHY Address 0 in non-broadcast" and it indeed behaves as such on KSZ8021. The issue where the ethernet controller (Freescale FEC) did not communicate with network is fixed by writing this bit as 1. Signed-off-by: Marek Vasut <marex@denx.de> Cc: David J. Choi <david.choi@micrel.com> Cc: David S. Miller <davem@davemloft.net> Cc: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com> Cc: Fabio Estevam <fabio.estevam@freescale.com> Cc: Shawn Guo <shawn.guo@linaro.org> --- drivers/net/phy/micrel.c | 27 +++++++++++++++++++++++++++ include/linux/micrel_phy.h | 1 + 2 files changed, 28 insertions(+) V2: Also add entry into micrel_tbl