diff mbox series

[net-next,5/7] net: phy: microchip_t1s: add support for Microchip's LAN867X Rev.C1

Message ID 20240812134816.380688-6-Parthiban.Veerasooran@microchip.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series microchip_t1s: Update on Microchip 10BASE-T1S PHY driver | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 29 this patch: 29
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 30 this patch: 30
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 35 this patch: 35
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 112 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Parthiban Veerasooran Aug. 12, 2024, 1:48 p.m. UTC 
This patch adds support for LAN8670/1/2 Rev.C1 as per the latest
configuration note AN1699 released (Revision E (DS60001699F - June 2024))
https://www.microchip.com/en-us/application-notes/an1699

Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
---
 drivers/net/phy/Kconfig         |  2 +-
 drivers/net/phy/microchip_t1s.c | 68 ++++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 3 deletions(-)

Comments

Simon Horman Aug. 15, 2024, 2:42 p.m. UTC | #1 
On Mon, Aug 12, 2024 at 07:18:14PM +0530, Parthiban Veerasooran wrote:
> This patch adds support for LAN8670/1/2 Rev.C1 as per the latest
> configuration note AN1699 released (Revision E (DS60001699F - June 2024))
> https://www.microchip.com/en-us/application-notes/an1699
> 
> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
> ---
>  drivers/net/phy/Kconfig         |  2 +-
>  drivers/net/phy/microchip_t1s.c | 68 ++++++++++++++++++++++++++++++++-
>  2 files changed, 67 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 68db15d52355..63b45544c191 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -282,7 +282,7 @@ config MICREL_PHY
>  config MICROCHIP_T1S_PHY
>  	tristate "Microchip 10BASE-T1S Ethernet PHYs"
>  	help
> -	  Currently supports the LAN8670/1/2 Rev.B1 and LAN8650/1 Rev.B0/B1
> +	  Currently supports the LAN8670/1/2 Rev.B1/C1 and LAN8650/1 Rev.B0/B1
>  	  Internal PHYs.
>  
>  config MICROCHIP_PHY
> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
> index d0af02a25d01..62f5ce548c6a 100644
> --- a/drivers/net/phy/microchip_t1s.c
> +++ b/drivers/net/phy/microchip_t1s.c
> @@ -3,7 +3,7 @@
>   * Driver for Microchip 10BASE-T1S PHYs
>   *
>   * Support: Microchip Phys:
> - *  lan8670/1/2 Rev.B1
> + *  lan8670/1/2 Rev.B1/C1
>   *  lan8650/1 Rev.B0/B1 Internal PHYs
>   */
>  
> @@ -12,6 +12,7 @@
>  #include <linux/phy.h>
>  
>  #define PHY_ID_LAN867X_REVB1 0x0007C162
> +#define PHY_ID_LAN867X_REVC1 0x0007C164
>  /* Both Rev.B0 and B1 clause 22 PHYID's are same due to B1 chip limitation */
>  #define PHY_ID_LAN865X_REVB 0x0007C1B3
>  
> @@ -243,7 +244,7 @@ static int lan865x_revb_config_init(struct phy_device *phydev)
>  		if (ret)
>  			return ret;
>  
> -		if (i == 2) {
> +		if (i == 1) {
>  			ret = lan865x_setup_cfgparam(phydev, offsets);
>  			if (ret)
>  				return ret;

Hi Parthiban,

This patch is addressing LAN867X Rev.C1 support.
But the hunk above appears to update LAN865X Rev.B0/B1 support.
Is that intentional?

...
Parthiban Veerasooran Aug. 16, 2024, 1:21 p.m. UTC | #2 
On 15/08/24 8:12 pm, Simon Horman wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Mon, Aug 12, 2024 at 07:18:14PM +0530, Parthiban Veerasooran wrote:
>> This patch adds support for LAN8670/1/2 Rev.C1 as per the latest
>> configuration note AN1699 released (Revision E (DS60001699F - June 2024))
>> https://www.microchip.com/en-us/application-notes/an1699
>>
>> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
>> ---
>>   drivers/net/phy/Kconfig         |  2 +-
>>   drivers/net/phy/microchip_t1s.c | 68 ++++++++++++++++++++++++++++++++-
>>   2 files changed, 67 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index 68db15d52355..63b45544c191 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -282,7 +282,7 @@ config MICREL_PHY
>>   config MICROCHIP_T1S_PHY
>>        tristate "Microchip 10BASE-T1S Ethernet PHYs"
>>        help
>> -       Currently supports the LAN8670/1/2 Rev.B1 and LAN8650/1 Rev.B0/B1
>> +       Currently supports the LAN8670/1/2 Rev.B1/C1 and LAN8650/1 Rev.B0/B1
>>          Internal PHYs.
>>
>>   config MICROCHIP_PHY
>> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
>> index d0af02a25d01..62f5ce548c6a 100644
>> --- a/drivers/net/phy/microchip_t1s.c
>> +++ b/drivers/net/phy/microchip_t1s.c
>> @@ -3,7 +3,7 @@
>>    * Driver for Microchip 10BASE-T1S PHYs
>>    *
>>    * Support: Microchip Phys:
>> - *  lan8670/1/2 Rev.B1
>> + *  lan8670/1/2 Rev.B1/C1
>>    *  lan8650/1 Rev.B0/B1 Internal PHYs
>>    */
>>
>> @@ -12,6 +12,7 @@
>>   #include <linux/phy.h>
>>
>>   #define PHY_ID_LAN867X_REVB1 0x0007C162
>> +#define PHY_ID_LAN867X_REVC1 0x0007C164
>>   /* Both Rev.B0 and B1 clause 22 PHYID's are same due to B1 chip limitation */
>>   #define PHY_ID_LAN865X_REVB 0x0007C1B3
>>
>> @@ -243,7 +244,7 @@ static int lan865x_revb_config_init(struct phy_device *phydev)
>>                if (ret)
>>                        return ret;
>>
>> -             if (i == 2) {
>> +             if (i == 1) {
>>                        ret = lan865x_setup_cfgparam(phydev, offsets);
>>                        if (ret)
>>                                return ret;
> 
> Hi Parthiban,
> 
> This patch is addressing LAN867X Rev.C1 support.
> But the hunk above appears to update LAN865X Rev.B0/B1 support.
> Is that intentional?

Hi Simon,

Sorry, there is a mistake here. It is supposed to be "i == 1" only, but 
it should have been in the below patch onwards,

"[PATCH net-next 2/7] net: phy: microchip_t1s: update new initial 
settings for LAN865X Rev.B0"

Thanks for pointing it out. Will correct it in the next version.

Best regards,
Parthiban V
> 
> ...
Simon Horman Aug. 16, 2024, 4:55 p.m. UTC | #3 
On Fri, Aug 16, 2024 at 01:21:02PM +0000, Parthiban.Veerasooran@microchip.com wrote:
> On 15/08/24 8:12 pm, Simon Horman wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Mon, Aug 12, 2024 at 07:18:14PM +0530, Parthiban Veerasooran wrote:
> >> This patch adds support for LAN8670/1/2 Rev.C1 as per the latest
> >> configuration note AN1699 released (Revision E (DS60001699F - June 2024))
> >> https://www.microchip.com/en-us/application-notes/an1699
> >>
> >> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
> >> ---
> >>   drivers/net/phy/Kconfig         |  2 +-
> >>   drivers/net/phy/microchip_t1s.c | 68 ++++++++++++++++++++++++++++++++-
> >>   2 files changed, 67 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> >> index 68db15d52355..63b45544c191 100644
> >> --- a/drivers/net/phy/Kconfig
> >> +++ b/drivers/net/phy/Kconfig
> >> @@ -282,7 +282,7 @@ config MICREL_PHY
> >>   config MICROCHIP_T1S_PHY
> >>        tristate "Microchip 10BASE-T1S Ethernet PHYs"
> >>        help
> >> -       Currently supports the LAN8670/1/2 Rev.B1 and LAN8650/1 Rev.B0/B1
> >> +       Currently supports the LAN8670/1/2 Rev.B1/C1 and LAN8650/1 Rev.B0/B1
> >>          Internal PHYs.
> >>
> >>   config MICROCHIP_PHY
> >> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
> >> index d0af02a25d01..62f5ce548c6a 100644
> >> --- a/drivers/net/phy/microchip_t1s.c
> >> +++ b/drivers/net/phy/microchip_t1s.c
> >> @@ -3,7 +3,7 @@
> >>    * Driver for Microchip 10BASE-T1S PHYs
> >>    *
> >>    * Support: Microchip Phys:
> >> - *  lan8670/1/2 Rev.B1
> >> + *  lan8670/1/2 Rev.B1/C1
> >>    *  lan8650/1 Rev.B0/B1 Internal PHYs
> >>    */
> >>
> >> @@ -12,6 +12,7 @@
> >>   #include <linux/phy.h>
> >>
> >>   #define PHY_ID_LAN867X_REVB1 0x0007C162
> >> +#define PHY_ID_LAN867X_REVC1 0x0007C164
> >>   /* Both Rev.B0 and B1 clause 22 PHYID's are same due to B1 chip limitation */
> >>   #define PHY_ID_LAN865X_REVB 0x0007C1B3
> >>
> >> @@ -243,7 +244,7 @@ static int lan865x_revb_config_init(struct phy_device *phydev)
> >>                if (ret)
> >>                        return ret;
> >>
> >> -             if (i == 2) {
> >> +             if (i == 1) {
> >>                        ret = lan865x_setup_cfgparam(phydev, offsets);
> >>                        if (ret)
> >>                                return ret;
> > 
> > Hi Parthiban,
> > 
> > This patch is addressing LAN867X Rev.C1 support.
> > But the hunk above appears to update LAN865X Rev.B0/B1 support.
> > Is that intentional?
> 
> Hi Simon,
> 
> Sorry, there is a mistake here. It is supposed to be "i == 1" only, but 
> it should have been in the below patch onwards,
> 
> "[PATCH net-next 2/7] net: phy: microchip_t1s: update new initial 
> settings for LAN865X Rev.B0"
> 
> Thanks for pointing it out. Will correct it in the next version.

Thanks,

Other than the minor problem noted above, the patchset looked good to me.
Parthiban Veerasooran Aug. 19, 2024, 7:24 a.m. UTC | #4 
Hi Simon,

On 16/08/24 10:25 pm, Simon Horman wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Aug 16, 2024 at 01:21:02PM +0000, Parthiban.Veerasooran@microchip.com wrote:
>> On 15/08/24 8:12 pm, Simon Horman wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Mon, Aug 12, 2024 at 07:18:14PM +0530, Parthiban Veerasooran wrote:
>>>> This patch adds support for LAN8670/1/2 Rev.C1 as per the latest
>>>> configuration note AN1699 released (Revision E (DS60001699F - June 2024))
>>>> https://www.microchip.com/en-us/application-notes/an1699
>>>>
>>>> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
>>>> ---
>>>>    drivers/net/phy/Kconfig         |  2 +-
>>>>    drivers/net/phy/microchip_t1s.c | 68 ++++++++++++++++++++++++++++++++-
>>>>    2 files changed, 67 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>>>> index 68db15d52355..63b45544c191 100644
>>>> --- a/drivers/net/phy/Kconfig
>>>> +++ b/drivers/net/phy/Kconfig
>>>> @@ -282,7 +282,7 @@ config MICREL_PHY
>>>>    config MICROCHIP_T1S_PHY
>>>>         tristate "Microchip 10BASE-T1S Ethernet PHYs"
>>>>         help
>>>> -       Currently supports the LAN8670/1/2 Rev.B1 and LAN8650/1 Rev.B0/B1
>>>> +       Currently supports the LAN8670/1/2 Rev.B1/C1 and LAN8650/1 Rev.B0/B1
>>>>           Internal PHYs.
>>>>
>>>>    config MICROCHIP_PHY
>>>> diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
>>>> index d0af02a25d01..62f5ce548c6a 100644
>>>> --- a/drivers/net/phy/microchip_t1s.c
>>>> +++ b/drivers/net/phy/microchip_t1s.c
>>>> @@ -3,7 +3,7 @@
>>>>     * Driver for Microchip 10BASE-T1S PHYs
>>>>     *
>>>>     * Support: Microchip Phys:
>>>> - *  lan8670/1/2 Rev.B1
>>>> + *  lan8670/1/2 Rev.B1/C1
>>>>     *  lan8650/1 Rev.B0/B1 Internal PHYs
>>>>     */
>>>>
>>>> @@ -12,6 +12,7 @@
>>>>    #include <linux/phy.h>
>>>>
>>>>    #define PHY_ID_LAN867X_REVB1 0x0007C162
>>>> +#define PHY_ID_LAN867X_REVC1 0x0007C164
>>>>    /* Both Rev.B0 and B1 clause 22 PHYID's are same due to B1 chip limitation */
>>>>    #define PHY_ID_LAN865X_REVB 0x0007C1B3
>>>>
>>>> @@ -243,7 +244,7 @@ static int lan865x_revb_config_init(struct phy_device *phydev)
>>>>                 if (ret)
>>>>                         return ret;
>>>>
>>>> -             if (i == 2) {
>>>> +             if (i == 1) {
>>>>                         ret = lan865x_setup_cfgparam(phydev, offsets);
>>>>                         if (ret)
>>>>                                 return ret;
>>>
>>> Hi Parthiban,
>>>
>>> This patch is addressing LAN867X Rev.C1 support.
>>> But the hunk above appears to update LAN865X Rev.B0/B1 support.
>>> Is that intentional?
>>
>> Hi Simon,
>>
>> Sorry, there is a mistake here. It is supposed to be "i == 1" only, but
>> it should have been in the below patch onwards,
>>
>> "[PATCH net-next 2/7] net: phy: microchip_t1s: update new initial
>> settings for LAN865X Rev.B0"
>>
>> Thanks for pointing it out. Will correct it in the next version.
> 
> Thanks,
> 
> Other than the minor problem noted above, the patchset looked good to me.
Thanks for your feedback. Its been almost a week since I posted this 
patch series and there are no other comments except this one. Shall I 
update this fix and post the next version of this patch series?

Best regards,
Parthiban V
>
diff mbox series

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 68db15d52355..63b45544c191 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -282,7 +282,7 @@  config MICREL_PHY
 config MICROCHIP_T1S_PHY
 	tristate "Microchip 10BASE-T1S Ethernet PHYs"
 	help
-	  Currently supports the LAN8670/1/2 Rev.B1 and LAN8650/1 Rev.B0/B1
+	  Currently supports the LAN8670/1/2 Rev.B1/C1 and LAN8650/1 Rev.B0/B1
 	  Internal PHYs.
 
 config MICROCHIP_PHY
diff --git a/drivers/net/phy/microchip_t1s.c b/drivers/net/phy/microchip_t1s.c
index d0af02a25d01..62f5ce548c6a 100644
--- a/drivers/net/phy/microchip_t1s.c
+++ b/drivers/net/phy/microchip_t1s.c
@@ -3,7 +3,7 @@ 
  * Driver for Microchip 10BASE-T1S PHYs
  *
  * Support: Microchip Phys:
- *  lan8670/1/2 Rev.B1
+ *  lan8670/1/2 Rev.B1/C1
  *  lan8650/1 Rev.B0/B1 Internal PHYs
  */
 
@@ -12,6 +12,7 @@ 
 #include <linux/phy.h>
 
 #define PHY_ID_LAN867X_REVB1 0x0007C162
+#define PHY_ID_LAN867X_REVC1 0x0007C164
 /* Both Rev.B0 and B1 clause 22 PHYID's are same due to B1 chip limitation */
 #define PHY_ID_LAN865X_REVB 0x0007C1B3
 
@@ -243,7 +244,7 @@  static int lan865x_revb_config_init(struct phy_device *phydev)
 		if (ret)
 			return ret;
 
-		if (i == 2) {
+		if (i == 1) {
 			ret = lan865x_setup_cfgparam(phydev, offsets);
 			if (ret)
 				return ret;
@@ -290,6 +291,58 @@  static int lan867x_check_reset_complete(struct phy_device *phydev)
 	return 0;
 }
 
+static int lan867x_revc1_config_init(struct phy_device *phydev)
+{
+	s8 offsets[2];
+	int ret;
+
+	ret = lan867x_check_reset_complete(phydev);
+	if (ret)
+		return ret;
+
+	ret = lan865x_generate_cfg_offsets(phydev, offsets);
+	if (ret)
+		return ret;
+
+	/* LAN867x Rev.C1 configuration settings are equal to the first 9
+	 * configuration settings and all the sqi fixup settings from LAN865x
+	 * Rev.B0/B1. So the same fixup registers and values from LAN865x
+	 * Rev.B0/B1 are used for LAN867x Rev.C1 to avoid duplication.
+	 * Refer the below links for the comparision.
+	 * https://www.microchip.com/en-us/application-notes/an1760
+	 * Revision F (DS60001760G - June 2024)
+	 * https://www.microchip.com/en-us/application-notes/an1699
+	 * Revision E (DS60001699F - June 2024)
+	 */
+	for (int i = 0; i < 9; i++) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
+				    lan865x_revb_fixup_registers[i],
+				    lan865x_revb_fixup_values[i]);
+		if (ret)
+			return ret;
+
+		if (i == 1) {
+			ret = lan865x_setup_cfgparam(phydev, offsets);
+			if (ret)
+				return ret;
+		}
+	}
+
+	ret = lan865x_setup_sqi_cfgparam(phydev, offsets);
+	if (ret)
+		return ret;
+
+	for (int i = 0; i < ARRAY_SIZE(lan865x_revb_sqi_fixup_regs); i++) {
+		ret = phy_write_mmd(phydev, MDIO_MMD_VEND2,
+				    lan865x_revb_sqi_fixup_regs[i],
+				    lan865x_revb_sqi_fixup_values[i]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int lan867x_revb1_config_init(struct phy_device *phydev)
 {
 	int err;
@@ -342,6 +395,16 @@  static struct phy_driver microchip_t1s_driver[] = {
 		.set_plca_cfg	    = genphy_c45_plca_set_cfg,
 		.get_plca_status    = genphy_c45_plca_get_status,
 	},
+	{
+		PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVC1),
+		.name               = "LAN867X Rev.C1",
+		.features           = PHY_BASIC_T1S_P2MP_FEATURES,
+		.config_init        = lan867x_revc1_config_init,
+		.read_status        = lan86xx_read_status,
+		.get_plca_cfg	    = genphy_c45_plca_get_cfg,
+		.set_plca_cfg	    = genphy_c45_plca_set_cfg,
+		.get_plca_status    = genphy_c45_plca_get_status,
+	},
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB),
 		.name               = "LAN865X Rev.B0/B1 Internal Phy",
@@ -358,6 +421,7 @@  module_phy_driver(microchip_t1s_driver);
 
 static struct mdio_device_id __maybe_unused tbl[] = {
 	{ PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVB1) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_LAN867X_REVC1) },
 	{ PHY_ID_MATCH_EXACT(PHY_ID_LAN865X_REVB) },
 	{ }
 };