Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1463869

Re: [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes

Show all headers | View raw

Hi Andrew,

Andrew Lunn <andrew@lunn.ch> writes:

> On Mon, Aug 15, 2016 at 05:19:01PM -0400, Vivien Didelot wrote:
>> +static int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
>> +				   u8 page, int reg, u16 *val)
>> +{
>> +	int err;
>> +
>> +	/* There is no paging for registers 22 */
>> +	if (reg == PHY_PAGE)
>> +		return -EINVAL;
>
> This whole paging scheme only works for internal PHYs, or external
> PHYs which happen to be Marvell PHYs. We need to be a little bit
> careful here and ensure these functions don't get used for external
> PHYs when we don't know who manufactured them. 
>
> At the moment the code is O.K, we only access SERDES or temperature
> sensors for a given port. But i wounder if adding a comment would be
> wise?

That is a good point, I thought about that too. I was thinking about
adding an internal_phys bitmask to the chip info structures and check it
in mv88e6xxx_phy_page_get(), so we could return -EINVAL for external
PHYs, since the switch driver isn't supposed to access their pages.

But I also think that most of the PHY code should be moved to a proper
PHY driver, since they are valid Marvell chips with their own PHY IDs.

Until we move the PHY and SERDES code out of the mv88e6xxx driver, I
think we are safe.

Thanks,

        Vivien

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH net-next 0/6] net: dsa: abstract PHY accesses Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
  [PATCH net-next 3/6] net: dsa: mv88e6xxx: rework Global2 SMI PHY access Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
  [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
    Re: [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and  SerDes Andrew Lunn <andrew@lunn.ch> - 2016-08-16 03:20 +0200
      Re: [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-16 17:00 +0200
  [PATCH net-next 6/6] net: dsa: mv88e6xxx: use the new PHY API Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
  [PATCH net-next 2/6] net: dsa: mv88e6xxx: describe Multi-chip registers Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
  [PATCH net-next 1/6] net: dsa: mv88e6xxx: rename _mv88e6xxx_wait Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
  [PATCH net-next 4/6] net: dsa: mv88e6xxx: abstract PHY ops Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-08-15 23:30 +0200
  Re: [PATCH net-next 0/6] net: dsa: abstract PHY accesses David Miller <davem@davemloft.net> - 2016-08-16 01:50 +0200

csiph-web