| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 Aug 2014 09:01:43 -0000
From: ygardi@...eaurora.org
To: "Kishon Vijay Abraham I" <kishon@...com>
Cc: ygardi@...eaurora.org, linux-arm-msm@...r.kernel,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
noag@...eaurora.org, subhashj@...eaurora.org,
gbroner@...eaurora.org
Subject: Re: [PATCH v1] phy: extend APIs of the generic phy framework
> Hi,
>
> On Tuesday 05 August 2014 12:03 AM, ygardi@...eaurora.org 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 <ygardi@...eaurora.org>
>>>> ---
>>>> 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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists