Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932388AbdLTIle (ORCPT ); Wed, 20 Dec 2017 03:41:34 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:57448 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932070AbdLTIlc (ORCPT ); Wed, 20 Dec 2017 03:41:32 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 9649C6070A Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=mgautam@codeaurora.org Subject: Re: [PATCH v3 14/16] phy: Add notify_speed callback To: Kishon Vijay Abraham I Cc: linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org, "open list:GENERIC PHY FRAMEWORK" 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> From: Manu Gautam Message-ID: <5b67c348-4ec9-58ca-05ed-8b93bed77efb@codeaurora.org> Date: Wed, 20 Dec 2017 14:11:27 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1781 Lines: 57 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. > Thanks > Kishon -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project