From: <Parthiban.Veerasooran@microchip•com>
To: <andrew@lunn•ch>
Cc: <netdev@vger•kernel.org>, <davem@davemloft•net>,
<Jan.Huber@microchip•com>, <Thorsten.Kummermehr@microchip•com>,
<ramon.nordin.rodriguez@ferroamp•se>
Subject: Re: [PATCH net-next 2/2] net: phy: microchip_t1s: add support for Microchip LAN865x Rev.B0 PHYs
Date: Thu, 27 Apr 2023 14:19:24 +0000 [thread overview]
Message-ID: <85f657bf-e2c6-0ad3-bac0-b87904697644@microchip.com> (raw)
In-Reply-To: <c9d06cfe-8834-4ffa-81a8-097c883cc960@lunn.ch>
On 27/04/23 3:22 am, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
>> -#define LAN867X_REG_IRQ_1_CTL 0x001C
>> -#define LAN867X_REG_IRQ_2_CTL 0x001D
>> +#define LAN86XX_REG_IRQ_1_CTL 0x001C
>> +#define LAN86XX_REG_IRQ_2_CTL 0x001D
>
> This patch is pretty big. Please split to LAN867X to LAN86XX rename
> out, to make the patches smaller and easier to reviewSure, will do it in the next version.
>
>> +static int lan865x_revb0_plca_set_cfg(struct phy_device *phydev,
>> + const struct phy_plca_cfg *plca_cfg)
>> +{
>> + int ret;
>> +
>> + ret = genphy_c45_plca_set_cfg(phydev, plca_cfg);
>> + if (ret)
>> + return ret;
>> +
>> + /* Disable the collision detection when PLCA is enabled and enable
>> + * collision detection when CSMA/CD mode is enabled.
>> + */
>> + if (plca_cfg->enabled)
>> + return phy_write_mmd(phydev, MDIO_MMD_VEND2, 0x0087, 0x0000);
>> + else
>> + return phy_write_mmd(phydev, MDIO_MMD_VEND2, 0x0087, 0x0083);
>> +}
>> +
>
> This could be in a patch of its own, with a good commit message
> explaining why it is needed.
Sure, as Ramon asked in his review comment this is also needed for
lan867x rev.b1 as well. So will make it as a separate patch before
lan865x rev.b0 patch.
>
> Once you replace the magic numbers with #defines, the comment becomes
> pointless. But what is missing is the answer to the question Why?
As I commented in previous email to Ramon, I am in contact with design
team for the clarification that why it is not documented. Will get more
info soon.
>
> Andrew
>
next prev parent reply other threads:[~2023-04-27 14:19 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-26 11:46 [PATCH net-next 0/2] add driver support for Microchip LAN865X Rev.B0 Internal PHYs Parthiban Veerasooran
2023-04-26 11:46 ` [PATCH net-next 1/2] net: phy: microchip_t1s: update LAN867x PHY supported revision number Parthiban Veerasooran
2023-04-26 20:56 ` Horatiu Vultur
2023-04-26 11:46 ` [PATCH net-next 2/2] net: phy: microchip_t1s: add support for Microchip LAN865x Rev.B0 PHYs Parthiban Veerasooran
2023-04-26 20:53 ` Ramón Nordin Rodriguez
2023-04-26 21:43 ` Andrew Lunn
2023-04-27 12:28 ` Parthiban.Veerasooran
2023-04-27 14:17 ` Parthiban.Veerasooran
2023-04-26 21:21 ` Horatiu Vultur
2023-04-27 11:28 ` Parthiban.Veerasooran
2023-04-26 21:52 ` Andrew Lunn
2023-04-27 14:19 ` Parthiban.Veerasooran [this message]
2023-04-26 22:02 ` Andrew Lunn
2023-04-27 14:23 ` Parthiban.Veerasooran
2023-04-26 20:50 ` [PATCH net-next 0/2] add driver support for Microchip LAN865X Rev.B0 Internal PHYs Horatiu Vultur
2023-04-27 7:07 ` Parthiban.Veerasooran
2023-04-27 11:24 ` Horatiu Vultur - M31836
2023-04-27 14:26 ` Parthiban.Veerasooran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=85f657bf-e2c6-0ad3-bac0-b87904697644@microchip.com \
--to=parthiban.veerasooran@microchip$(echo .)com \
--cc=Jan.Huber@microchip$(echo .)com \
--cc=Thorsten.Kummermehr@microchip$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ramon.nordin.rodriguez@ferroamp$(echo .)se \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox