diff mbox

[U-Boot,v3,3/4] drivers: phy: add generic_phy_valid() method

Message ID 1495533475-1553-4-git-send-email-patrice.chotard@st.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Patrice CHOTARD May 23, 2017, 9:57 a.m. UTC 
From: Patrice Chotard <patrice.chotard@st.com>

This allow to check if a PHY has been correctly
initialised and avoid to get access to phy struct.

Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/phy/phy-uclass.c | 5 +++++
 include/generic-phy.h    | 8 ++++++++
 2 files changed, 13 insertions(+)

Comments

Jean-Jacques Hiblot May 24, 2017, 1:24 p.m. UTC | #1 
On 23/05/2017 11:57, patrice.chotard@st.com wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
>
> This allow to check if a PHY has been correctly
> initialised and avoid to get access to phy struct.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>   drivers/phy/phy-uclass.c | 5 +++++
>   include/generic-phy.h    | 8 ++++++++
>   2 files changed, 13 insertions(+)
>
> diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
> index 0d8bef7..43a7c18 100644
> --- a/drivers/phy/phy-uclass.c
> +++ b/drivers/phy/phy-uclass.c
> @@ -133,6 +133,11 @@ int generic_phy_power_off(struct phy *phy)
>   	return ops->power_off ? ops->power_off(phy) : 0;
>   }
>   
> +bool generic_phy_valid(struct phy *phy)
> +{
> +	return phy->dev != NULL;
> +}
> +
Not that it has big impact but I would have made it an inline function

Also note that there is a loophole in generic_phy_get_by_index() that 
needs to be fixed before phy->dev can be used to check if the phy is 
valid; phy->dev must be set to NULL if generic_phy_get_by_index() fails.

Jean-Jacques

>   UCLASS_DRIVER(phy) = {
>   	.id		= UCLASS_PHY,
>   	.name		= "phy",
> diff --git a/include/generic-phy.h b/include/generic-phy.h
> index d8cf0c9..53206f0 100644
> --- a/include/generic-phy.h
> +++ b/include/generic-phy.h
> @@ -221,4 +221,12 @@ int generic_phy_get_by_index(struct udevice *user, int index,
>   int generic_phy_get_by_name(struct udevice *user, const char *phy_name,
>   			    struct phy *phy);
>   
> +/**
> + * generic_phy_valid() - check if PHY port is valid
> + *
> + * @phy:	the PHY port to check
> + * @return TRUE if valid, or FALSE
> + */
> +bool generic_phy_valid(struct phy *phy);
> +
>   #endif /*__GENERIC_PHY_H */
Marek Vasut May 24, 2017, 3:08 p.m. UTC | #2 
On 05/24/2017 03:24 PM, Jean-Jacques Hiblot wrote:
> 
> 
> On 23/05/2017 11:57, patrice.chotard@st.com wrote:
>> From: Patrice Chotard <patrice.chotard@st.com>
>>
>> This allow to check if a PHY has been correctly
>> initialised and avoid to get access to phy struct.
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>   drivers/phy/phy-uclass.c | 5 +++++
>>   include/generic-phy.h    | 8 ++++++++
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
>> index 0d8bef7..43a7c18 100644
>> --- a/drivers/phy/phy-uclass.c
>> +++ b/drivers/phy/phy-uclass.c
>> @@ -133,6 +133,11 @@ int generic_phy_power_off(struct phy *phy)
>>       return ops->power_off ? ops->power_off(phy) : 0;
>>   }
>>   +bool generic_phy_valid(struct phy *phy)
>> +{
>> +    return phy->dev != NULL;
>> +}
>> +
> Not that it has big impact but I would have made it an inline function

The inline is just a hint , the compiler can decide on that itself.

> Also note that there is a loophole in generic_phy_get_by_index() that
> needs to be fixed before phy->dev can be used to check if the phy is
> valid; phy->dev must be set to NULL if generic_phy_get_by_index() fails.
> 
> Jean-Jacques
> 
>>   UCLASS_DRIVER(phy) = {
>>       .id        = UCLASS_PHY,
>>       .name        = "phy",
>> diff --git a/include/generic-phy.h b/include/generic-phy.h
>> index d8cf0c9..53206f0 100644
>> --- a/include/generic-phy.h
>> +++ b/include/generic-phy.h
>> @@ -221,4 +221,12 @@ int generic_phy_get_by_index(struct udevice
>> *user, int index,
>>   int generic_phy_get_by_name(struct udevice *user, const char *phy_name,
>>                   struct phy *phy);
>>   +/**
>> + * generic_phy_valid() - check if PHY port is valid
>> + *
>> + * @phy:    the PHY port to check
>> + * @return TRUE if valid, or FALSE
>> + */
>> +bool generic_phy_valid(struct phy *phy);
>> +
>>   #endif /*__GENERIC_PHY_H */
> 
>
Patrice CHOTARD May 29, 2017, 8:01 a.m. UTC | #3 
Hi Jean-Jacques

On 05/24/2017 03:24 PM, Jean-Jacques Hiblot wrote:
> 

> 

> On 23/05/2017 11:57, patrice.chotard@st.com wrote:

>> From: Patrice Chotard <patrice.chotard@st.com>

>>

>> This allow to check if a PHY has been correctly

>> initialised and avoid to get access to phy struct.

>>

>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>

>> ---

>>   drivers/phy/phy-uclass.c | 5 +++++

>>   include/generic-phy.h    | 8 ++++++++

>>   2 files changed, 13 insertions(+)

>>

>> diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c

>> index 0d8bef7..43a7c18 100644

>> --- a/drivers/phy/phy-uclass.c

>> +++ b/drivers/phy/phy-uclass.c

>> @@ -133,6 +133,11 @@ int generic_phy_power_off(struct phy *phy)

>>       return ops->power_off ? ops->power_off(phy) : 0;

>>   }

>> +bool generic_phy_valid(struct phy *phy)

>> +{

>> +    return phy->dev != NULL;

>> +}

>> +

> Not that it has big impact but I would have made it an inline function

> 

> Also note that there is a loophole in generic_phy_get_by_index() that 

> needs to be fixed before phy->dev can be used to check if the phy is 

> valid; phy->dev must be set to NULL if generic_phy_get_by_index() fails.


Thanks for pointing this.
I will send a patch fixing this and include it in the v4 of my series " 
Extend xhci-dwc3"

Thanks

Patrice

> 

> Jean-Jacques

> 

>>   UCLASS_DRIVER(phy) = {

>>       .id        = UCLASS_PHY,

>>       .name        = "phy",

>> diff --git a/include/generic-phy.h b/include/generic-phy.h

>> index d8cf0c9..53206f0 100644

>> --- a/include/generic-phy.h

>> +++ b/include/generic-phy.h

>> @@ -221,4 +221,12 @@ int generic_phy_get_by_index(struct udevice 

>> *user, int index,

>>   int generic_phy_get_by_name(struct udevice *user, const char *phy_name,

>>                   struct phy *phy);

>> +/**

>> + * generic_phy_valid() - check if PHY port is valid

>> + *

>> + * @phy:    the PHY port to check

>> + * @return TRUE if valid, or FALSE

>> + */

>> +bool generic_phy_valid(struct phy *phy);

>> +

>>   #endif /*__GENERIC_PHY_H */

> 

>
Simon Glass June 1, 2017, 3:11 a.m. UTC | #4 
On 23 May 2017 at 03:57,  <patrice.chotard@st.com> wrote:
> From: Patrice Chotard <patrice.chotard@st.com>
>
> This allow to check if a PHY has been correctly
> initialised and avoid to get access to phy struct.
>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>  drivers/phy/phy-uclass.c | 5 +++++
>  include/generic-phy.h    | 8 ++++++++
>  2 files changed, 13 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>
diff mbox

Patch

diff --git a/drivers/phy/phy-uclass.c b/drivers/phy/phy-uclass.c
index 0d8bef7..43a7c18 100644
--- a/drivers/phy/phy-uclass.c
+++ b/drivers/phy/phy-uclass.c
@@ -133,6 +133,11 @@  int generic_phy_power_off(struct phy *phy)
 	return ops->power_off ? ops->power_off(phy) : 0;
 }
 
+bool generic_phy_valid(struct phy *phy)
+{
+	return phy->dev != NULL;
+}
+
 UCLASS_DRIVER(phy) = {
 	.id		= UCLASS_PHY,
 	.name		= "phy",
diff --git a/include/generic-phy.h b/include/generic-phy.h
index d8cf0c9..53206f0 100644
--- a/include/generic-phy.h
+++ b/include/generic-phy.h
@@ -221,4 +221,12 @@  int generic_phy_get_by_index(struct udevice *user, int index,
 int generic_phy_get_by_name(struct udevice *user, const char *phy_name,
 			    struct phy *phy);
 
+/**
+ * generic_phy_valid() - check if PHY port is valid
+ *
+ * @phy:	the PHY port to check
+ * @return TRUE if valid, or FALSE
+ */
+bool generic_phy_valid(struct phy *phy);
+
 #endif /*__GENERIC_PHY_H */