Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752535AbdIVSPv (ORCPT ); Fri, 22 Sep 2017 14:15:51 -0400 Received: from mail.savoirfairelinux.com ([208.88.110.44]:47562 "EHLO mail.savoirfairelinux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751980AbdIVSPt (ORCPT ); Fri, 22 Sep 2017 14:15:49 -0400 From: Vivien Didelot To: Florian Fainelli , netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com, "David S. Miller" , Andrew Lunn Subject: Re: [PATCH net-next 2/4] net: dsa: remove phy arg from port enable/disable In-Reply-To: References: <20170922161753.19563-1-vivien.didelot@savoirfairelinux.com> <20170922161753.19563-3-vivien.didelot@savoirfairelinux.com> Date: Fri, 22 Sep 2017 14:12:14 -0400 Message-ID: <87377eob5t.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me> 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 Content-Length: 1234 Lines: 33 Hi Florian, Florian Fainelli writes: > On 09/22/2017 09:17 AM, Vivien Didelot wrote: >> The .port_enable and .port_disable functions are meant to deal with the >> switch ports only, and no driver is using the phy argument anyway. >> Remove it. > > I don't think this makes sense, there are perfectly legit reasons why a > switch driver may have something to do with the PHY device attached to > its per-port network interface, we should definitively keep that around, > unless you think we should be accessing the PHY within the switch > drivers by doing: > > struct phy_device *phydev = ds->ports[port].netdev->phydev? bcm_sf2 is the only user for this phy argument right now. The reason I'm doing this is because I prefer to discourage switch drivers to dig into the phy device themselves while as you said there must be a cleaner solution. This must be handled somehow elsewhere in the stack. In the meantime, moving the PHY device up to the dsa_port structure is a good solution, in order not to expose it in switch ops, but still make it available to more complex drivers. Do you know if netdev->phydev is usable? Why do DSA has its own copy in dsa_slave_priv then? I'll respin, thanks. Vivien