Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753424AbdL1LEU (ORCPT ); Thu, 28 Dec 2017 06:04:20 -0500 Received: from lelnx193.ext.ti.com ([198.47.27.77]:10366 "EHLO lelnx193.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752905AbdL1LES (ORCPT ); Thu, 28 Dec 2017 06:04:18 -0500 Subject: Re: [PATCH v3 14/16] phy: Add notify_speed callback To: Manu Gautam References: <1511256206-1587-1-git-send-email-mgautam@codeaurora.org> <1511256206-1587-15-git-send-email-mgautam@codeaurora.org> <082d2ca8-21dc-878f-c668-a76872a7ea92@ti.com> <5b67c348-4ec9-58ca-05ed-8b93bed77efb@codeaurora.org> CC: , , "open list:GENERIC PHY FRAMEWORK" From: Kishon Vijay Abraham I Message-ID: Date: Thu, 28 Dec 2017 16:34:11 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <5b67c348-4ec9-58ca-05ed-8b93bed77efb@codeaurora.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1854 Lines: 58 Hi, On Wednesday 20 December 2017 02:11 PM, Manu Gautam wrote: > Hi > > > On 12/20/2017 12:47 PM, Kishon Vijay Abraham I wrote: >> Hi, >> > [snip] >>>> Why not use a notification mechanism instead of adding new APIs in phy-core. >>>> This will only bloat phy-core with APIs for a particular platform. >>> Do you mean notifier_chains ? >>> When we have multiple instances of USB PHYs then notifier chains are not >>> of much help. For any platform glue or PHY driver it will be very difficult to >>> figure out if notification received for speed was for same phy/bus or a >>> different one. >>> Using PHY callbacks looked more elegant to me. Additionally PHY drivers >>> can also use this info decide power management policy e.g. if speed is >>> INVALID then it means PHY is not in a session and it can enter deepest >>> low power state. >>> Additionally if you prefer set_speed name over notify_speed then I am >>> ok with that as well so that it sounds more generic. >> I'd prefer adding modes in enum phy_mode according to speed and using phy_set_mode. > > yeah, that also seems good idea. How about something like this: > > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -23,12 +23,16 @@ > struct phy; > > enum phy_mode { > - PHY_MODE_INVALID, > - PHY_MODE_USB_HOST, > - PHY_MODE_USB_DEVICE, > - PHY_MODE_USB_OTG, > - PHY_MODE_SGMII, > - PHY_MODE_10GKR, > + PHY_MODE_INVALID = 0, > + PHY_MODE_USB_HOST = BIT(0), > + PHY_MODE_USB_DEVICE = BIT(1), > + PHY_MODE_USB_OTG, = BIT(2), > + PHY_MODE_SGMII = BIT(3), > + PHY_MODE_10GKR = BIT(4), > + PHY_MODE_USB_LS = BIT(5), > + PHY_MODE_USB_FS = BIT(6), > + PHY_MODE_USB_HS = BIT(7), > + PHY_MODE_USB_SS = BIT(8), > }; > > > This way I don't need to duplicate USB speed enums for host/device or otg modes. no.. let's keep enum. It's lot more cleaner IMO. Thanks Kishon