phy: core: document calibrate() method
diff mbox series

Message ID 20190719095245.17401-1-m.szyprowski@samsung.com
State Not Applicable
Headers show
Series
  • phy: core: document calibrate() method
Related show

Commit Message

Marek Szyprowski July 19, 2019, 9:52 a.m. UTC
Commit 36914111e682 ("drivers: phy: add calibrate method") added support
for generic phy_calibrate() method, but it didn't explain in detail when
such method is supposed to be called. Add some more documentation directly
to the phy.h to make it clean that it is intended to be called after every
host controller reset.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 include/linux/phy/phy.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Kishon Vijay Abraham I July 19, 2019, 10:04 a.m. UTC | #1
Hi Marek,

On 19/07/19 3:22 PM, Marek Szyprowski wrote:
> Commit 36914111e682 ("drivers: phy: add calibrate method") added support
> for generic phy_calibrate() method, but it didn't explain in detail when
> such method is supposed to be called. Add some more documentation directly
> to the phy.h to make it clean that it is intended to be called after every
> host controller reset.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  include/linux/phy/phy.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index 15032f145063..46775e8b0ed9 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -101,6 +101,18 @@ struct phy_ops {
>  	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
>  			    union phy_configure_opts *opts);
>  	int	(*reset)(struct phy *phy);
> +
> +	/**
> +	 * @calibrate:
> +	 *
> +	 * Optional.
> +	 *
> +	 * Used to calibrate phy, typically by adjusting some parameters
> +	 * in runtime, which are otherwise lost after host controller
> +	 * reset and cannot be set in phy_init() and phy_power_on().
> +	 *
> +	 * Returns: 0 if successful, an negative error code otherwise
> +	 */
>  	int	(*calibrate)(struct phy *phy);

This should be added in drivers/phy/phy-core.c before phy_calibrate()? We could
add a separate section in Documentation/phy.txt to document these phy_ops.

Thanks
Kishon
Marek Szyprowski July 19, 2019, 12:21 p.m. UTC | #2
Hi Kishon,

On 2019-07-19 12:04, Kishon Vijay Abraham I wrote:
> Hi Marek,
>
> On 19/07/19 3:22 PM, Marek Szyprowski wrote:
>> Commit 36914111e682 ("drivers: phy: add calibrate method") added support
>> for generic phy_calibrate() method, but it didn't explain in detail when
>> such method is supposed to be called. Add some more documentation directly
>> to the phy.h to make it clean that it is intended to be called after every
>> host controller reset.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   include/linux/phy/phy.h | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>> index 15032f145063..46775e8b0ed9 100644
>> --- a/include/linux/phy/phy.h
>> +++ b/include/linux/phy/phy.h
>> @@ -101,6 +101,18 @@ struct phy_ops {
>>   	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
>>   			    union phy_configure_opts *opts);
>>   	int	(*reset)(struct phy *phy);
>> +
>> +	/**
>> +	 * @calibrate:
>> +	 *
>> +	 * Optional.
>> +	 *
>> +	 * Used to calibrate phy, typically by adjusting some parameters
>> +	 * in runtime, which are otherwise lost after host controller
>> +	 * reset and cannot be set in phy_init() and phy_power_on().
>> +	 *
>> +	 * Returns: 0 if successful, an negative error code otherwise
>> +	 */
>>   	int	(*calibrate)(struct phy *phy);
> This should be added in drivers/phy/phy-core.c before phy_calibrate()?

Okay. Do you want to keep it also in phy.h (like for validate and 
configure)?


> We could
> add a separate section in Documentation/phy.txt to document these phy_ops.

Best regards
Kishon Vijay Abraham I July 19, 2019, 12:25 p.m. UTC | #3
Hi Marek,

On 19/07/19 5:51 PM, Marek Szyprowski wrote:
> Hi Kishon,
> 
> On 2019-07-19 12:04, Kishon Vijay Abraham I wrote:
>> Hi Marek,
>>
>> On 19/07/19 3:22 PM, Marek Szyprowski wrote:
>>> Commit 36914111e682 ("drivers: phy: add calibrate method") added support
>>> for generic phy_calibrate() method, but it didn't explain in detail when
>>> such method is supposed to be called. Add some more documentation directly
>>> to the phy.h to make it clean that it is intended to be called after every
>>> host controller reset.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>   include/linux/phy/phy.h | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>>> index 15032f145063..46775e8b0ed9 100644
>>> --- a/include/linux/phy/phy.h
>>> +++ b/include/linux/phy/phy.h
>>> @@ -101,6 +101,18 @@ struct phy_ops {
>>>   	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
>>>   			    union phy_configure_opts *opts);
>>>   	int	(*reset)(struct phy *phy);
>>> +
>>> +	/**
>>> +	 * @calibrate:
>>> +	 *
>>> +	 * Optional.
>>> +	 *
>>> +	 * Used to calibrate phy, typically by adjusting some parameters
>>> +	 * in runtime, which are otherwise lost after host controller
>>> +	 * reset and cannot be set in phy_init() and phy_power_on().
>>> +	 *
>>> +	 * Returns: 0 if successful, an negative error code otherwise
>>> +	 */
>>>   	int	(*calibrate)(struct phy *phy);
>> This should be added in drivers/phy/phy-core.c before phy_calibrate()?
> 
> Okay. Do you want to keep it also in phy.h (like for validate and 
> configure)?

No. Adding in phy.txt and before phy_calibrate() should suffice IMO.

Thanks
Kishon
Marek Szyprowski July 22, 2019, 4:50 p.m. UTC | #4
Hi Kishon,

On 2019-07-19 14:25, Kishon Vijay Abraham I wrote:
> On 19/07/19 5:51 PM, Marek Szyprowski wrote:
>> On 2019-07-19 12:04, Kishon Vijay Abraham I wrote:
>>> On 19/07/19 3:22 PM, Marek Szyprowski wrote:
>>>> Commit 36914111e682 ("drivers: phy: add calibrate method") added support
>>>> for generic phy_calibrate() method, but it didn't explain in detail when
>>>> such method is supposed to be called. Add some more documentation directly
>>>> to the phy.h to make it clean that it is intended to be called after every
>>>> host controller reset.
>>>>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>>    include/linux/phy/phy.h | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
>>>> index 15032f145063..46775e8b0ed9 100644
>>>> --- a/include/linux/phy/phy.h
>>>> +++ b/include/linux/phy/phy.h
>>>> @@ -101,6 +101,18 @@ struct phy_ops {
>>>>    	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
>>>>    			    union phy_configure_opts *opts);
>>>>    	int	(*reset)(struct phy *phy);
>>>> +
>>>> +	/**
>>>> +	 * @calibrate:
>>>> +	 *
>>>> +	 * Optional.
>>>> +	 *
>>>> +	 * Used to calibrate phy, typically by adjusting some parameters
>>>> +	 * in runtime, which are otherwise lost after host controller
>>>> +	 * reset and cannot be set in phy_init() and phy_power_on().
>>>> +	 *
>>>> +	 * Returns: 0 if successful, an negative error code otherwise
>>>> +	 */
>>>>    	int	(*calibrate)(struct phy *phy);
>>> This should be added in drivers/phy/phy-core.c before phy_calibrate()?
>> Okay. Do you want to keep it also in phy.h (like for validate and
>> configure)?
> No. Adding in phy.txt and before phy_calibrate() should suffice IMO.

OKay, I will move it to phy.c. Generic PHYs documentation is converted 
to RST (Documentation/driver-api/phy/phy.rst), so it could simply 
include those comments from .c file. This can be done in separate patch.

Best regards

Patch
diff mbox series

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 15032f145063..46775e8b0ed9 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -101,6 +101,18 @@  struct phy_ops {
 	int	(*validate)(struct phy *phy, enum phy_mode mode, int submode,
 			    union phy_configure_opts *opts);
 	int	(*reset)(struct phy *phy);
+
+	/**
+	 * @calibrate:
+	 *
+	 * Optional.
+	 *
+	 * Used to calibrate phy, typically by adjusting some parameters
+	 * in runtime, which are otherwise lost after host controller
+	 * reset and cannot be set in phy_init() and phy_power_on().
+	 *
+	 * Returns: 0 if successful, an negative error code otherwise
+	 */
 	int	(*calibrate)(struct phy *phy);
 	void	(*release)(struct phy *phy);
 	struct module *owner;