[PATCH 1/4] PCI: introduce helper functions for device flag operation
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_clear_dev_assigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
v2: simplify unnecessory ternary operation in function pci_is_dev_assigned();
v3: amend helper functions naming.
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 466bcd1..b610ab3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1829,4 +1829,17 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
*/
struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_clear_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED;
+}
#endif /* LINUX_PCI_H */
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t pcibios_window_alignment(struct
pci_bus *bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED ? true : false;
+}
The ternary operation isn't necessary. Thanks,
Alex
/* kmem_cache style wrapper around pci_alloc_consistent() */
#include linux/pci-dma.h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
On 2014/7/29 5:00, Alex Williamson wrote:
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t pcibios_window_alignment(struct pci_bus
*bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
Though it is a flag operation now, may not later, we define it
because we want to hide the internal operation.
'set' to 'deassigned' status is enough. So I would like keep it.
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED ? true : false;
+}
The ternary operation isn't necessary. Thanks,
Yep,
return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED
is enough.
Thanks,
Ethan
Alex
/* kmem_cache style wrapper around pci_alloc_consistent() */
#include linux/pci-dma.h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
On 2014/7/29 5:00, Alex Williamson wrote:
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t pcibios_window_alignment(struct
pci_bus *bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
Though it is a flag operation now, may not later, we define it
because we want to hide the internal operation.
'set' to 'deassigned' status is enough. So I would like keep it.
I disagree, the opposite of a 'set' is a 'clear', or at least an
'unset'. Using bit-ops-like terminology doesn't lock us into an
implementation. As written, this could just as easily be setting two
different variables.
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED ? true : false;
+}
The ternary operation isn't necessary. Thanks,
Yep,
return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED
is enough.
Thanks,
Ethan
Alex
/* kmem_cache style wrapper around pci_alloc_consistent() */
#include linux/pci-dma.h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
On 2014/7/29 10:31, Alex Williamson wrote:
On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
On 2014/7/29 5:00, Alex Williamson wrote:
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t pcibios_window_alignment(struct pci_bus
*bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
Though it is a flag operation now, may not later, we define it
because we want to hide the internal operation.
'set' to 'deassigned' status is enough. So I would like keep it.
I disagree, the opposite of a 'set' is a 'clear', or at least an
'unset'. Using bit-ops-like terminology doesn't lock us into an
implementation. As written, this could just as easily be setting two
different variables.
So there are two pairs of opposite:
set assigned --- unset assigned
set assigned --- set deassigned
Here you prefer the 'verb' set /unset, and I prefer the 'adj.'
assigned / deassigned.
Do they really have different meaning or make confusion ? I don't
think so.
Thanks,
Ethan
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED ? true : false;
+}
The ternary operation isn't necessary. Thanks,
Yep,
return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED
is enough.
Thanks,
Ethan
Alex
/* kmem_cache style wrapper around pci_alloc_consistent() */
#include linux/pci-dma.h
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
On 07/28/2014 07:43 PM, ethan zhao wrote:
On 2014/7/29 10:31, Alex Williamson wrote:
On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
On 2014/7/29 5:00, Alex Williamson wrote:
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t
pcibios_window_alignment(struct pci_bus *bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
Though it is a flag operation now, may not later, we define it
because we want to hide the internal operation.
'set' to 'deassigned' status is enough. So I would like keep it.
I disagree, the opposite of a 'set' is a 'clear', or at least an
'unset'. Using bit-ops-like terminology doesn't lock us into an
implementation. As written, this could just as easily be setting two
different variables.
So there are two pairs of opposite:
set assigned --- unset assigned
set assigned --- set deassigned
Here you prefer the 'verb' set /unset, and I prefer the 'adj.' assigned
/ deassigned.
Do they really have different meaning or make confusion ? I don't think
so.
Thanks,
Ethan
I agree with Alex W. If you are going to use the set name you should
probably use clear for the operation that undoes it. Using the term
set implies that it is setting a bit and we currently don't have a
deassigned/unassigned bit.
If that doesn't work perhaps you could use something like
get/put_assigned_device or acquire/release_assigned_device. You just
need to describe what it is you are doing. You could even consider
adding some tests to return an error if you attempt to assign a device
that is already assigned.
Thanks,
Alex D
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] PCI: introduce helper functions for device flag operation
Both of you and Alex W prefer the 'Verb' , Ok, I accept the suggestion.
Thanks,
Ethan
On Tue, Jul 29, 2014 at 11:37 AM, Alexander Duyck
alexander.du...@gmail.com wrote:
On 07/28/2014 07:43 PM, ethan zhao wrote:
On 2014/7/29 10:31, Alex Williamson wrote:
On Tue, 2014-07-29 at 09:53 +0800, ethan zhao wrote:
On 2014/7/29 5:00, Alex Williamson wrote:
On Wed, 2014-07-23 at 00:19 +0800, Ethan Zhao wrote:
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t
pcibios_window_alignment(struct pci_bus *bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
I think pci_clear_dev_assigned() would make more sense, we're not
setting a flag named DEASSIGNED.
Though it is a flag operation now, may not later, we define it
because we want to hide the internal operation.
'set' to 'deassigned' status is enough. So I would like keep it.
I disagree, the opposite of a 'set' is a 'clear', or at least an
'unset'. Using bit-ops-like terminology doesn't lock us into an
implementation. As written, this could just as easily be setting two
different variables.
So there are two pairs of opposite:
set assigned --- unset assigned
set assigned --- set deassigned
Here you prefer the 'verb' set /unset, and I prefer the 'adj.' assigned
/ deassigned.
Do they really have different meaning or make confusion ? I don't think
so.
Thanks,
Ethan
I agree with Alex W. If you are going to use the set name you should
probably use clear for the operation that undoes it. Using the term
set implies that it is setting a bit and we currently don't have a
deassigned/unassigned bit.
If that doesn't work perhaps you could use something like
get/put_assigned_device or acquire/release_assigned_device. You just
need to describe what it is you are doing. You could even consider
adding some tests to return an error if you attempt to assign a device
that is already assigned.
Thanks,
Alex D
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] PCI: introduce helper functions for device flag operation
This patch introduced three helper functions to hide direct
device flag operation.
void pci_set_dev_assigned(struct pci_dev *pdev);
void pci_set_dev_deassigned(struct pci_dev *pdev);
bool pci_is_dev_assigned(struct pci_dev *pdev);
Signed-off-by: Ethan Zhao ethan.z...@oracle.com
---
include/linux/pci.h | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index aab57b4..5f6f8fa 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1129,6 +1129,19 @@ resource_size_t pcibios_window_alignment(struct pci_bus
*bus,
int pci_set_vga_state(struct pci_dev *pdev, bool decode,
unsigned int command_bits, u32 flags);
+/* helper functions for operation of device flag */
+static inline void pci_set_dev_assigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline void pci_set_dev_deassigned(struct pci_dev *pdev)
+{
+ pdev-dev_flags = ~PCI_DEV_FLAGS_ASSIGNED;
+}
+static inline bool pci_is_dev_assigned(struct pci_dev *pdev)
+{
+ return pdev-dev_flags PCI_DEV_FLAGS_ASSIGNED ? true : false;
+}
/* kmem_cache style wrapper around pci_alloc_consistent() */
#include linux/pci-dma.h
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
