Re: [PATCH 02/14] of/pci: Add of_pci_get_devfn() function

2013-01-10 Thread Thierry Reding 
On Thu, Jan 10, 2013 at 05:09:43PM -0700, Stephen Warren wrote:
> On 01/09/2013 01:43 PM, Thierry Reding wrote:
> > This function can be used to parse the device and function number from a
> > standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
> > the returned value obtain the device and function numbers respectively.
> 
> > diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> 
> >  static inline int __of_pci_pci_compare(struct device_node *node,
> >unsigned int devfn)
> >  {
> > +   int err;
> >  
> > +   err = of_pci_get_devfn(node);
> > +   if (err < 0)
> > return 0;
> > +
> > +   return devfn == err;
> 
> I know this is really picky, but calling that "err" when it's hopefully
> not an error but rather a PCI device/function ID seems a little obscure.
> Perhaps node_devfn? I assume that fact that devfn is unsigned and err is
> signed won't be an issue.

Maybe renaming the devfn parameter to data and using devfn for the local
variable would be more obvious.

The signedness shouldn't be problematic. devfn is an 8-bit unsigned
integer and sign mismatch is excluded by the error return already.

Thierry


pgpSd96sMkIJ5.pgp
Description: PGP signature

Re: [PATCH 02/14] of/pci: Add of_pci_get_devfn() function

2013-01-10 Thread Stephen Warren 
On 01/09/2013 01:43 PM, Thierry Reding wrote:
> This function can be used to parse the device and function number from a
> standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
> the returned value obtain the device and function numbers respectively.

> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c

>  static inline int __of_pci_pci_compare(struct device_node *node,
>  unsigned int devfn)
>  {
> + int err;
>  
> + err = of_pci_get_devfn(node);
> + if (err < 0)
>   return 0;
> +
> + return devfn == err;

I know this is really picky, but calling that "err" when it's hopefully
not an error but rather a PCI device/function ID seems a little obscure.
Perhaps node_devfn? I assume that fact that devfn is unsigned and err is
signed won't be an issue.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 02/14] of/pci: Add of_pci_get_devfn() function

2013-01-10 Thread Stephen Warren 
On 01/09/2013 01:43 PM, Thierry Reding wrote:
 This function can be used to parse the device and function number from a
 standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
 the returned value obtain the device and function numbers respectively.

 diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c

  static inline int __of_pci_pci_compare(struct device_node *node,
  unsigned int devfn)
  {
 + int err;
  
 + err = of_pci_get_devfn(node);
 + if (err  0)
   return 0;
 +
 + return devfn == err;

I know this is really picky, but calling that err when it's hopefully
not an error but rather a PCI device/function ID seems a little obscure.
Perhaps node_devfn? I assume that fact that devfn is unsigned and err is
signed won't be an issue.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 02/14] of/pci: Add of_pci_get_devfn() function

2013-01-10 Thread Thierry Reding 
On Thu, Jan 10, 2013 at 05:09:43PM -0700, Stephen Warren wrote:
 On 01/09/2013 01:43 PM, Thierry Reding wrote:
  This function can be used to parse the device and function number from a
  standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
  the returned value obtain the device and function numbers respectively.
 
  diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
 
   static inline int __of_pci_pci_compare(struct device_node *node,
 unsigned int devfn)
   {
  +   int err;
   
  +   err = of_pci_get_devfn(node);
  +   if (err  0)
  return 0;
  +
  +   return devfn == err;
 
 I know this is really picky, but calling that err when it's hopefully
 not an error but rather a PCI device/function ID seems a little obscure.
 Perhaps node_devfn? I assume that fact that devfn is unsigned and err is
 signed won't be an issue.

Maybe renaming the devfn parameter to data and using devfn for the local
variable would be more obvious.

The signedness shouldn't be problematic. devfn is an 8-bit unsigned
integer and sign mismatch is excluded by the error return already.

Thierry


pgpSd96sMkIJ5.pgp
Description: PGP signature

[PATCH 02/14] of/pci: Add of_pci_get_devfn() function

2013-01-09 Thread Thierry Reding 
This function can be used to parse the device and function number from a
standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
the returned value obtain the device and function numbers respectively.

Signed-off-by: Thierry Reding 
---
 drivers/of/of_pci.c| 32 
 include/linux/of_pci.h |  1 +
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 13e37e2..0dd52df 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -7,12 +7,13 @@
 static inline int __of_pci_pci_compare(struct device_node *node,
   unsigned int devfn)
 {
-   unsigned int size;
-   const __be32 *reg = of_get_property(node, "reg", );
+   int err;
 
-   if (!reg || size < 5 * sizeof(__be32))
+   err = of_pci_get_devfn(node);
+   if (err < 0)
return 0;
-   return ((be32_to_cpup([0]) >> 8) & 0xff) == devfn;
+
+   return devfn == err;
 }
 
 struct device_node *of_pci_find_child_device(struct device_node *parent,
@@ -40,3 +41,26 @@ struct device_node *of_pci_find_child_device(struct 
device_node *parent,
return NULL;
 }
 EXPORT_SYMBOL_GPL(of_pci_find_child_device);
+
+/**
+ * of_pci_get_devfn() - Get device and function numbers for a device node
+ * @np: device node
+ *
+ * Parses a standard 5-cell PCI resource and returns an 8-bit value that can
+ * be passed to the PCI_SLOT() and PCI_FUNC() macros to extract the device
+ * and function numbers respectively. On error a negative error code is
+ * returned.
+ */
+int of_pci_get_devfn(struct device_node *np)
+{
+   unsigned int size;
+   const __be32 *reg;
+
+   reg = of_get_property(np, "reg", );
+
+   if (!reg || size < 5 * sizeof(__be32))
+   return -EINVAL;
+
+   return (be32_to_cpup(reg) >> 8) & 0xff;
+}
+EXPORT_SYMBOL_GPL(of_pci_get_devfn);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bb115de..91ec484 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -10,5 +10,6 @@ int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq 
*out_irq);
 struct device_node;
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 unsigned int devfn);
+int of_pci_get_devfn(struct device_node *np);
 
 #endif
-- 
1.8.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 02/14] of/pci: Add of_pci_get_devfn() function

2013-01-09 Thread Thierry Reding 
This function can be used to parse the device and function number from a
standard 5-cell PCI resource. PCI_SLOT() and PCI_FUNC() can be used on
the returned value obtain the device and function numbers respectively.

Signed-off-by: Thierry Reding thierry.red...@avionic-design.de
---
 drivers/of/of_pci.c| 32 
 include/linux/of_pci.h |  1 +
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 13e37e2..0dd52df 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -7,12 +7,13 @@
 static inline int __of_pci_pci_compare(struct device_node *node,
   unsigned int devfn)
 {
-   unsigned int size;
-   const __be32 *reg = of_get_property(node, reg, size);
+   int err;
 
-   if (!reg || size  5 * sizeof(__be32))
+   err = of_pci_get_devfn(node);
+   if (err  0)
return 0;
-   return ((be32_to_cpup(reg[0])  8)  0xff) == devfn;
+
+   return devfn == err;
 }
 
 struct device_node *of_pci_find_child_device(struct device_node *parent,
@@ -40,3 +41,26 @@ struct device_node *of_pci_find_child_device(struct 
device_node *parent,
return NULL;
 }
 EXPORT_SYMBOL_GPL(of_pci_find_child_device);
+
+/**
+ * of_pci_get_devfn() - Get device and function numbers for a device node
+ * @np: device node
+ *
+ * Parses a standard 5-cell PCI resource and returns an 8-bit value that can
+ * be passed to the PCI_SLOT() and PCI_FUNC() macros to extract the device
+ * and function numbers respectively. On error a negative error code is
+ * returned.
+ */
+int of_pci_get_devfn(struct device_node *np)
+{
+   unsigned int size;
+   const __be32 *reg;
+
+   reg = of_get_property(np, reg, size);
+
+   if (!reg || size  5 * sizeof(__be32))
+   return -EINVAL;
+
+   return (be32_to_cpup(reg)  8)  0xff;
+}
+EXPORT_SYMBOL_GPL(of_pci_get_devfn);
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index bb115de..91ec484 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -10,5 +10,6 @@ int of_irq_map_pci(const struct pci_dev *pdev, struct of_irq 
*out_irq);
 struct device_node;
 struct device_node *of_pci_find_child_device(struct device_node *parent,
 unsigned int devfn);
+int of_pci_get_devfn(struct device_node *np);
 
 #endif
-- 
1.8.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/