> Hi, > > On Tuesday 05 August 2014 12:03 AM, [email protected] wrote: >>> Hi, >>> >>> On Thursday 31 July 2014 06:07 PM, Yaniv Gardi wrote: >>>> This change adds a few more APIs to the phy_ops structure: >>>> advertise_quirks - API for setting the phy quirks >>> >>> What are these phy quirks? An explanation on what you are are planning >>> to >>> do >>> with these quirks might help. >> >> A phy HW might have a specific behavior that should be exposed to the >> phy >> SW structure by calling this callback. This is the reason behind this >> callback. does it make more sense now ? > > I'm more interested in the _specific_ behaviour you are talking about. > Such > callbacks can be abused easily.
I'm not sure I understand how this callback can be abused more than any other callback. however, specific behaviour of PHY can be, for example, different configuration of attributes during power_on power_off when a quirk of hibern8_exit is advertised etc. >> >> >>>> suspend - API for the implementation of phy suspend sequence >>>> resume - API for the implementation of phy resume sequence >>> >>> Why not use the existing pm_runtime's suspend/resume callbacks? >>> >> Like we observed in our case, often there are need to be extra >> operations >> in the suspend/resume sequence. those specific operations include >> turning >> off/on specific clocks, disabling/enabling regulators etc. the specific >> implementation must give enough flexibility to add whatever needed to be >> done in suspend/resume sequences. > > All these can be done in suspend and resume callbacks of pm_runtime. > > Thanks > Kishon >> >> >> >> >>>> >>>> Change-Id: I44dd77f2603d20acb02ccb0cc0d20ade884f97c2 >>> Remove this.. >>> >>>> Signed-off-by: Yaniv Gardi <[email protected]> >>>> --- >>>> Documentation/phy.txt | 6 ++--- >>>> drivers/phy/phy-core.c | 58 >>>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> include/linux/phy/phy.h | 33 ++++++++++++++++++++++++++++ >>>> 3 files changed, 94 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/phy.txt b/Documentation/phy.txt >>>> index c6594af..f0dc28e 100644 >>>> --- a/Documentation/phy.txt >>>> +++ b/Documentation/phy.txt >>>> @@ -63,9 +63,9 @@ struct phy *devm_phy_create(struct device *dev, >>>> struct >>>> device_node *node, >>>> The PHY drivers can use one of the above 2 APIs to create the PHY by >>>> passing >>>> the device pointer, phy ops and init_data. >>>> phy_ops is a set of function pointers for performing PHY operations >>>> such as >>>> -init, exit, power_on and power_off. *init_data* is mandatory to get a >>>> reference >>>> -to the PHY in the case of non-dt boot. See section *Board File >>>> Initialization* >>>> -on how init_data should be used. >>>> +init, exit, power_on and power_off, , suspend, resume and >>>> advertise_quirks. >>>> +*init_data* is mandatory to get a reference to the PHY in the case of >>>> non-dt >>>> +boot. See section *Board File Initialization* on how init_data should >>>> be used. >>>> >>>> Inorder to dereference the private data (in phy_ops), the phy >>>> provider >>>> driver >>>> can use phy_set_drvdata() after creating the PHY and use >>>> phy_get_drvdata() in >>>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c >>>> index ff5eec5..77abaab 100644 >>>> --- a/drivers/phy/phy-core.c >>>> +++ b/drivers/phy/phy-core.c >>>> @@ -293,6 +293,64 @@ int phy_power_off(struct phy *phy) >>>> } >>>> EXPORT_SYMBOL_GPL(phy_power_off); >>>> >>>> +int phy_suspend(struct phy *phy) >>>> +{ >>>> + int ret = 0; >>>> + >>>> + if (!phy->ops->suspend) >>>> + return -ENOTSUPP; >>>> + >>>> + mutex_lock(&phy->mutex); >>>> + >>>> + if (--phy->resume_count == 0) { >>>> + ret = phy->ops->suspend(phy); >>>> + if (ret) { >>>> + dev_err(&phy->dev, "phy suspend failed --> %d\n", ret); >>>> + /* reverting the resume_count since suspend failed */ >>>> + phy->resume_count++; >>>> + goto out; >>>> + } >>>> + } >>>> +out: >>>> + mutex_unlock(&phy->mutex); >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL(phy_suspend); >>>> + >>>> +int phy_resume(struct phy *phy) >>>> +{ >>>> + int ret = 0; >>>> + >>>> + if (!phy->ops->resume) >>>> + return -ENOTSUPP; >>>> + >>>> + mutex_lock(&phy->mutex); >>>> + >>>> + if (phy->resume_count++ == 0) { >>>> + ret = phy->ops->resume(phy); >>>> + if (ret) { >>>> + dev_err(&phy->dev, "phy resume failed --> %d\n", ret); >>>> + /* reverting the resume_count since resume failed */ >>>> + phy->resume_count--; >>>> + goto out; >>>> + } >>>> + } >>>> +out: >>>> + mutex_unlock(&phy->mutex); >>>> + return ret; >>>> +} >>>> +EXPORT_SYMBOL(phy_resume); >>>> + >>>> +void phy_advertise_quirks(struct phy *phy) >>>> +{ >>>> + if (phy->ops->advertise_quirks) { >>>> + mutex_lock(&phy->mutex); >>>> + phy->ops->advertise_quirks(phy); >>>> + mutex_unlock(&phy->mutex); >>>> + } >>>> +} >>>> +EXPORT_SYMBOL(phy_advertise_quirks); >>>> + >>>> /** >>>> * _of_phy_get() - lookup and obtain a reference to a phy by phandle >>>> * @np: device_node for which to get the phy >>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h >>>> index 8cb6f81..5b96d65 100644 >>>> --- a/include/linux/phy/phy.h >>>> +++ b/include/linux/phy/phy.h >>>> @@ -28,6 +28,14 @@ struct phy; >>>> * @exit: operation to be performed while exiting >>>> * @power_on: powering on the phy >>>> * @power_off: powering off the phy >>>> + * @advertise_quirks: setting specific phy quirks. this api is for an >>>> + internal use of the device driver, and its >>>> + purpose is to exteriorize the driver's phy quirks >>>> + according to phy version (or other parameters), >>>> + so further behaviour of the driver's phy is based >>>> + on those quirks. >>> >>> Can you be more specific on what you do with this? This looks more like >>> a >>> candidate for flags than callback to me? >>> >>> Thanks >>> Kishon >>> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
