Subject: Re: [PATCH] net: phy: dp83848: Add TI DP83848 Ethernet PHY

From: "Andrew F. Davis" <[email-masked]>
Date: Tue, 20 Oct 2015 16:51:19 -0500

On 10/20/2015 04:37 PM, Florian Fainelli wrote:
> On 20/10/15 14:28, Andrew F. Davis wrote:
>> Add support for the TI DP83848 Ethernet PHY device.
>>
>> The DP83848 is a highly reliable, feature rich, IEEE 802.3 compliant
>> single port 10/100 Mb/s Ethernet Physical Layer Transceiver supporting
>> the MII and RMII interfaces.
>>
>> Signed-off-by: Andrew F. Davis <[email-masked]>
>> Signed-off-by: Dan Murphy <[email-masked]>
>
> Reviewed-by: Florian Fainelli <[email-masked]>
>
> Just one comment below:
>
> [snip]
>
>> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
>> index 87f079c..b748224 100644
>> --- a/drivers/net/phy/Makefile
>> +++ b/drivers/net/phy/Makefile
>> @@ -24,6 +24,7 @@ obj-$(CONFIG_MDIO_BITBANG)	+= mdio-bitbang.o
>>   obj-$(CONFIG_MDIO_GPIO)		+= mdio-gpio.o
>>   obj-$(CONFIG_NATIONAL_PHY)	+= national.o
>>   obj-$(CONFIG_DP83640_PHY)	+= dp83640.o
>> +obj-$(CONFIG_DP83848_PHY)	+= dp83848.o
>>   obj-$(CONFIG_DP83867_PHY)	+= dp83867.o
>
> This is a pretty small PHY driver, would it make sense to look into
> consolidating these into e.g: dp838xx.c for instance? dp83640 is big
> enough to be its own driver it seems.
>

I was looking into that, but from the looks of it these drivers are small
because only the functions that differentiate the PHYs need be defined.
So there is very little common code between them.

I'm going to be brining up a couple more PHYs soon, so I'll keep this in mind
and try to find any redundancy that can be merged when working on these.

Thanks,
Andrew