From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753531AbcHPO6C (ORCPT ); Tue, 16 Aug 2016 10:58:02 -0400 Received: from mail.savoirfairelinux.com ([208.88.110.44]:36142 "EHLO mail.savoirfairelinux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752499AbcHPO6A (ORCPT ); Tue, 16 Aug 2016 10:58:00 -0400 From: Vivien Didelot To: Andrew Lunn Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com, "David S. Miller" , Florian Fainelli Subject: Re: [PATCH net-next 5/6] net: dsa: mv88e6xxx: describe PHY page and SerDes In-Reply-To: <20160816011618.GL24664@lunn.ch> References: <20160815211902.2236-1-vivien.didelot@savoirfairelinux.com> <20160815211902.2236-6-vivien.didelot@savoirfairelinux.com> <20160816011618.GL24664@lunn.ch> User-Agent: Notmuch/0.22.1 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-unknown-linux-gnu) Date: Tue, 16 Aug 2016 10:57:56 -0400 Message-ID: <874m6kzrpn.fsf@ketchup.mtl.sfl> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andrew, Andrew Lunn 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