[PATCH 0/2] Fixup changes to pci_epf_type_add_cfs()

[PATCH 0/2] Fixup changes to pci_epf_type_add_cfs()

[Damien Le Moal]

A couple of patches to improve and document the changes to
pci_epf_type_add_cfs() done with patch
893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code").

Damien Le Moal (2):
  PCI: endpoint: Improve pci_epf_type_add_cfs()
  PCI: endpoint: Add kdoc description of pci_epf_type_add_cfs()

 drivers/pci/endpoint/pci-ep-cfs.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

-- 
2.40.1

[PATCH 1/2] PCI: endpoint: Improve pci_epf_type_add_cfs()

[Damien Le Moal]

pci_epf_type_add_cfs() should not be called with an unbound EPF device,
that is, an epf device with epf->driver not set. For such case, replace
the NULL return in pci_epf_type_add_cfs() with a clear ERR_PTR(-EINVAL)
error return.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/pci/endpoint/pci-ep-cfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
index 0fb6c376166f..d8a6abe2c31c 100644
--- a/drivers/pci/endpoint/pci-ep-cfs.c
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -516,7 +516,7 @@ static struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
 
 	if (!epf->driver) {
 		dev_err(&epf->dev, "epf device not bound to driver\n");
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 
 	if (!epf->driver->ops->add_cfs)
-- 
2.40.1

[PATCH 2/2] PCI: endpoint: Add kdoc description of pci_epf_type_add_cfs()

[Damien Le Moal]

Restore an improve the kdoc function description for
pci_epf_type_add_cfs() that was removed with commit
893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code").

Fixes: 893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/pci/endpoint/pci-ep-cfs.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
index d8a6abe2c31c..15e17a661875 100644
--- a/drivers/pci/endpoint/pci-ep-cfs.c
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -509,6 +509,24 @@ static const struct config_item_type pci_epf_type = {
 	.ct_owner	= THIS_MODULE,
 };
 
+/**
+ * pci_epf_type_add_cfs() - Help function drivers to expose function specific   
+ *                          attributes in configfs
+ * @epf: the EPF device that has to be configured using configfs
+ * @group: the parent configfs group (corresponding to entries in
+ *         pci_epf_device_id)
+ *
+ * Invoke to expose function specific attributes in configfs.
+ *
+ * Return: NULL if the function driver
+ *
+ * Return: A pointer to a config_group structure or NULL if the function driver
+ * does not have anything to expose (attributes configured by user) or if the
+ * the function driver does not implement the add_cfs() method.
+ *
+ * Returns an error pointer if this function is called for an unbound EPF device
+ * or if the EPF driver add_cfs() method fails.
+ */
 static struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
 						 struct config_group *group)
 {
-- 
2.40.1

Re: [PATCH 1/2] PCI: endpoint: Improve pci_epf_type_add_cfs()

[Manivannan Sadhasivami]

On Mon, May 15, 2023 at 02:14:59PM +0900, Damien Le Moal wrote:
> pci_epf_type_add_cfs() should not be called with an unbound EPF device,
> that is, an epf device with epf->driver not set. For such case, replace
> the NULL return in pci_epf_type_add_cfs() with a clear ERR_PTR(-EINVAL)
> error return.
> 

Shouldn't the error code be -ENODEV?

- Mani

> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/pci/endpoint/pci-ep-cfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index 0fb6c376166f..d8a6abe2c31c 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -516,7 +516,7 @@ static struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
>  
>  	if (!epf->driver) {
>  		dev_err(&epf->dev, "epf device not bound to driver\n");
> -		return NULL;
> +		return ERR_PTR(-EINVAL);
>  	}
>  
>  	if (!epf->driver->ops->add_cfs)
> -- 
> 2.40.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 2/2] PCI: endpoint: Add kdoc description of pci_epf_type_add_cfs()

[Manivannan Sadhasivami]

On Mon, May 15, 2023 at 02:15:00PM +0900, Damien Le Moal wrote:
> Restore an improve the kdoc function description for
> pci_epf_type_add_cfs() that was removed with commit
> 893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code").
> 
> Fixes: 893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/pci/endpoint/pci-ep-cfs.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index d8a6abe2c31c..15e17a661875 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -509,6 +509,24 @@ static const struct config_item_type pci_epf_type = {
>  	.ct_owner	= THIS_MODULE,
>  };
>  
> +/**
> + * pci_epf_type_add_cfs() - Help function drivers to expose function specific   
> + *                          attributes in configfs
> + * @epf: the EPF device that has to be configured using configfs
> + * @group: the parent configfs group (corresponding to entries in
> + *         pci_epf_device_id)
> + *
> + * Invoke to expose function specific attributes in configfs.
> + *
> + * Return: NULL if the function driver

Spurious?

- Mani

> + *
> + * Return: A pointer to a config_group structure or NULL if the function driver
> + * does not have anything to expose (attributes configured by user) or if the
> + * the function driver does not implement the add_cfs() method.
> + *
> + * Returns an error pointer if this function is called for an unbound EPF device
> + * or if the EPF driver add_cfs() method fails.
> + */
>  static struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
>  						 struct config_group *group)
>  {
> -- 
> 2.40.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/2] PCI: endpoint: Improve pci_epf_type_add_cfs()

[Damien Le Moal]

On 5/15/23 16:19, Manivannan Sadhasivami wrote:
> On Mon, May 15, 2023 at 02:14:59PM +0900, Damien Le Moal wrote:
>> pci_epf_type_add_cfs() should not be called with an unbound EPF device,
>> that is, an epf device with epf->driver not set. For such case, replace
>> the NULL return in pci_epf_type_add_cfs() with a clear ERR_PTR(-EINVAL)
>> error return.
>>
> 
> Shouldn't the error code be -ENODEV?

Yeah, I wondered about it and settled for EINVAL... Will change to ENODEV.

> 
> - Mani
> 
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>> ---
>>  drivers/pci/endpoint/pci-ep-cfs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
>> index 0fb6c376166f..d8a6abe2c31c 100644
>> --- a/drivers/pci/endpoint/pci-ep-cfs.c
>> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
>> @@ -516,7 +516,7 @@ static struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
>>  
>>  	if (!epf->driver) {
>>  		dev_err(&epf->dev, "epf device not bound to driver\n");
>> -		return NULL;
>> +		return ERR_PTR(-EINVAL);
>>  	}
>>  
>>  	if (!epf->driver->ops->add_cfs)
>> -- 
>> 2.40.1
>>
> 

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] PCI: endpoint: Add kdoc description of pci_epf_type_add_cfs()

[Damien Le Moal]

On 5/15/23 16:21, Manivannan Sadhasivami wrote:
> On Mon, May 15, 2023 at 02:15:00PM +0900, Damien Le Moal wrote:
>> Restore an improve the kdoc function description for
>> pci_epf_type_add_cfs() that was removed with commit
>> 893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code").
>>
>> Fixes: 893f14fed7d3 ("PCI: endpoint: Move pci_epf_type_add_cfs() code")
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>> ---
>>  drivers/pci/endpoint/pci-ep-cfs.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
>> index d8a6abe2c31c..15e17a661875 100644
>> --- a/drivers/pci/endpoint/pci-ep-cfs.c
>> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
>> @@ -509,6 +509,24 @@ static const struct config_item_type pci_epf_type = {
>>  	.ct_owner	= THIS_MODULE,
>>  };
>>  
>> +/**
>> + * pci_epf_type_add_cfs() - Help function drivers to expose function specific   
>> + *                          attributes in configfs
>> + * @epf: the EPF device that has to be configured using configfs
>> + * @group: the parent configfs group (corresponding to entries in
>> + *         pci_epf_device_id)
>> + *
>> + * Invoke to expose function specific attributes in configfs.
>> + *
>> + * Return: NULL if the function driver
> 
> Spurious?

Oops. Indeed. Fixing this.

> 
> - Mani
> 
>> + *
>> + * Return: A pointer to a config_group structure or NULL if the function driver
>> + * does not have anything to expose (attributes configured by user) or if the
>> + * the function driver does not implement the add_cfs() method.
>> + *
>> + * Returns an error pointer if this function is called for an unbound EPF device
>> + * or if the EPF driver add_cfs() method fails.
>> + */
>>  static struct config_group *pci_epf_type_add_cfs(struct pci_epf *epf,
>>  						 struct config_group *group)
>>  {
>> -- 
>> 2.40.1
>>
> 

-- 
Damien Le Moal
Western Digital Research