diff mbox series

[1/3] hw/qdev: Restrict qdev_get_gpio_out_connector() to qdev-internal.h

Message ID 20211229225206.171882-2-philmd@redhat.com (mailing list archive)
State New, archived
Headers show
Series hw/sysbus: Document GPIO related functions | expand

Commit Message

Philippe Mathieu-Daudé Dec. 29, 2021, 10:52 p.m. UTC 
qdev_get_gpio_out_connector() is called by sysbus_get_connected_irq()
which is only used by platform-bus.c; restrict it to hw/core/ by
adding a local "qdev-internal.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/qdev-internal.h | 15 +++++++++++++++
 include/hw/qdev-core.h  | 18 ------------------
 hw/core/gpio.c          |  1 +
 hw/core/sysbus.c        |  1 +
 4 files changed, 17 insertions(+), 18 deletions(-)
 create mode 100644 hw/core/qdev-internal.h

Comments

Zhijian Li (Fujitsu)" via Dec. 31, 2021, 7:30 a.m. UTC | #1 
Hi,

On 2021/12/30 6:52, Philippe Mathieu-Daudé wrote:
> qdev_get_gpio_out_connector() is called by sysbus_get_connected_irq()
> which is only used by platform-bus.c; restrict it to hw/core/ by
> adding a local "qdev-internal.h" header.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   hw/core/qdev-internal.h | 15 +++++++++++++++
>   include/hw/qdev-core.h  | 18 ------------------
>   hw/core/gpio.c          |  1 +
>   hw/core/sysbus.c        |  1 +
>   4 files changed, 17 insertions(+), 18 deletions(-)
>   create mode 100644 hw/core/qdev-internal.h
>
> diff --git a/hw/core/qdev-internal.h b/hw/core/qdev-internal.h
> new file mode 100644
> index 00000000000..6ec17d0ea70
> --- /dev/null
> +++ b/hw/core/qdev-internal.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * qdev internal helpers
> + *
> + * Copyright (c) 2009-2021 QEMU contributors
> + */
> +#ifndef HW_CORE_QDEV_INTERNAL_H
> +#define HW_CORE_QDEV_INTERNAL_H
> +
> +#include "hw/qdev-core.h"
> +
> +/* Following functions are only used by the platform-bus subsystem */
Could it be better to also keep the original function comment here?
> +qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
> +
> +#endif /* HW_CORE_QDEV_INTERNAL_H */
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index d19c9417520..655899654bb 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -532,24 +532,6 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>   void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
>                                    qemu_irq input_pin);
>   
> -/**
> - * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
> - * @dev: Device whose output GPIO we are interested in
> - * @name: Name of the output GPIO array
> - * @n: Number of the output GPIO line within that array
> - *
> - * Returns whatever qemu_irq is currently connected to the specified
> - * output GPIO line of @dev. This will be NULL if the output GPIO line
> - * has never been wired up to the anything.  Note that the qemu_irq
> - * returned does not belong to @dev -- it will be the input GPIO or
> - * IRQ of whichever device the board code has connected up to @dev's
> - * output GPIO.
> - *
> - * You probably don't need to use this function -- it is used only
> - * by the platform-bus subsystem.
> - */
> -qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
> -
>   /**
>    * qdev_intercept_gpio_out: Intercept an existing GPIO connection
>    * @dev: Device to intercept the outbound GPIO line from
> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
> index 80d07a6ec99..513ccbd1062 100644
> --- a/hw/core/gpio.c
> +++ b/hw/core/gpio.c
> @@ -21,6 +21,7 @@
>   #include "hw/qdev-core.h"
>   #include "hw/irq.h"
>   #include "qapi/error.h"
> +#include "qdev-internal.h"
>   
>   static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
>                                                  const char *name)
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 05c1da3d311..0e6773c8df7 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -23,6 +23,7 @@
>   #include "hw/sysbus.h"
>   #include "monitor/monitor.h"
>   #include "exec/address-spaces.h"
> +#include "qdev-internal.h"
>   
>   static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
>   static char *sysbus_get_fw_dev_path(DeviceState *dev);
Otherwise, the tweak looks reasonable:
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
Philippe Mathieu-Daudé Dec. 31, 2021, 12:11 p.m. UTC | #2 
On 12/31/21 08:30, wangyanan (Y) wrote:
> Hi,
> 
> On 2021/12/30 6:52, Philippe Mathieu-Daudé wrote:
>> qdev_get_gpio_out_connector() is called by sysbus_get_connected_irq()
>> which is only used by platform-bus.c; restrict it to hw/core/ by
>> adding a local "qdev-internal.h" header.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   hw/core/qdev-internal.h | 15 +++++++++++++++
>>   include/hw/qdev-core.h  | 18 ------------------
>>   hw/core/gpio.c          |  1 +
>>   hw/core/sysbus.c        |  1 +
>>   4 files changed, 17 insertions(+), 18 deletions(-)
>>   create mode 100644 hw/core/qdev-internal.h
>>
>> diff --git a/hw/core/qdev-internal.h b/hw/core/qdev-internal.h
>> new file mode 100644
>> index 00000000000..6ec17d0ea70
>> --- /dev/null
>> +++ b/hw/core/qdev-internal.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * qdev internal helpers
>> + *
>> + * Copyright (c) 2009-2021 QEMU contributors
>> + */
>> +#ifndef HW_CORE_QDEV_INTERNAL_H
>> +#define HW_CORE_QDEV_INTERNAL_H
>> +
>> +#include "hw/qdev-core.h"
>> +
>> +/* Following functions are only used by the platform-bus subsystem */
> Could it be better to also keep the original function comment here?

We could, but this include being now internal, it seems superfluous.

Since Peter documented this function, let see if he has an preference.

>> +qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char
>> *name, int n);
>> +
>> +#endif /* HW_CORE_QDEV_INTERNAL_H */
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index d19c9417520..655899654bb 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -532,24 +532,6 @@ void qdev_connect_gpio_out(DeviceState *dev, int
>> n, qemu_irq pin);
>>   void qdev_connect_gpio_out_named(DeviceState *dev, const char *name,
>> int n,
>>                                    qemu_irq input_pin);
>>   -/**
>> - * qdev_get_gpio_out_connector: Get the qemu_irq connected to an
>> output GPIO
>> - * @dev: Device whose output GPIO we are interested in
>> - * @name: Name of the output GPIO array
>> - * @n: Number of the output GPIO line within that array
>> - *
>> - * Returns whatever qemu_irq is currently connected to the specified
>> - * output GPIO line of @dev. This will be NULL if the output GPIO line
>> - * has never been wired up to the anything.  Note that the qemu_irq
>> - * returned does not belong to @dev -- it will be the input GPIO or
>> - * IRQ of whichever device the board code has connected up to @dev's
>> - * output GPIO.
>> - *
>> - * You probably don't need to use this function -- it is used only
>> - * by the platform-bus subsystem.
>> - */
>> -qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char
>> *name, int n);
>> -
>>   /**
>>    * qdev_intercept_gpio_out: Intercept an existing GPIO connection
>>    * @dev: Device to intercept the outbound GPIO line from
>> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
>> index 80d07a6ec99..513ccbd1062 100644
>> --- a/hw/core/gpio.c
>> +++ b/hw/core/gpio.c
>> @@ -21,6 +21,7 @@
>>   #include "hw/qdev-core.h"
>>   #include "hw/irq.h"
>>   #include "qapi/error.h"
>> +#include "qdev-internal.h"
>>     static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
>>                                                  const char *name)
>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index 05c1da3d311..0e6773c8df7 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -23,6 +23,7 @@
>>   #include "hw/sysbus.h"
>>   #include "monitor/monitor.h"
>>   #include "exec/address-spaces.h"
>> +#include "qdev-internal.h"
>>     static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int
>> indent);
>>   static char *sysbus_get_fw_dev_path(DeviceState *dev);
> Otherwise, the tweak looks reasonable:
> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,

Phil.
Zhijian Li (Fujitsu)" via Jan. 3, 2022, 9:15 a.m. UTC | #3 
On 2021/12/31 20:11, Philippe Mathieu-Daudé wrote:
> On 12/31/21 08:30, wangyanan (Y) wrote:
>> Hi,
>>
>> On 2021/12/30 6:52, Philippe Mathieu-Daudé wrote:
>>> qdev_get_gpio_out_connector() is called by sysbus_get_connected_irq()
>>> which is only used by platform-bus.c; restrict it to hw/core/ by
>>> adding a local "qdev-internal.h" header.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>    hw/core/qdev-internal.h | 15 +++++++++++++++
>>>    include/hw/qdev-core.h  | 18 ------------------
>>>    hw/core/gpio.c          |  1 +
>>>    hw/core/sysbus.c        |  1 +
>>>    4 files changed, 17 insertions(+), 18 deletions(-)
>>>    create mode 100644 hw/core/qdev-internal.h
>>>
>>> diff --git a/hw/core/qdev-internal.h b/hw/core/qdev-internal.h
>>> new file mode 100644
>>> index 00000000000..6ec17d0ea70
>>> --- /dev/null
>>> +++ b/hw/core/qdev-internal.h
>>> @@ -0,0 +1,15 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> +/*
>>> + * qdev internal helpers
>>> + *
>>> + * Copyright (c) 2009-2021 QEMU contributors
>>> + */
>>> +#ifndef HW_CORE_QDEV_INTERNAL_H
>>> +#define HW_CORE_QDEV_INTERNAL_H
>>> +
>>> +#include "hw/qdev-core.h"
>>> +
>>> +/* Following functions are only used by the platform-bus subsystem */
>> Could it be better to also keep the original function comment here?
> We could, but this include being now internal, it seems superfluous.
Ok, makes sense to me. :)

Thanks,
Yanan
> Since Peter documented this function, let see if he has an preference.
>
>>> +qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char
>>> *name, int n);
>>> +
>>> +#endif /* HW_CORE_QDEV_INTERNAL_H */
>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>> index d19c9417520..655899654bb 100644
>>> --- a/include/hw/qdev-core.h
>>> +++ b/include/hw/qdev-core.h
>>> @@ -532,24 +532,6 @@ void qdev_connect_gpio_out(DeviceState *dev, int
>>> n, qemu_irq pin);
>>>    void qdev_connect_gpio_out_named(DeviceState *dev, const char *name,
>>> int n,
>>>                                     qemu_irq input_pin);
>>>    -/**
>>> - * qdev_get_gpio_out_connector: Get the qemu_irq connected to an
>>> output GPIO
>>> - * @dev: Device whose output GPIO we are interested in
>>> - * @name: Name of the output GPIO array
>>> - * @n: Number of the output GPIO line within that array
>>> - *
>>> - * Returns whatever qemu_irq is currently connected to the specified
>>> - * output GPIO line of @dev. This will be NULL if the output GPIO line
>>> - * has never been wired up to the anything.  Note that the qemu_irq
>>> - * returned does not belong to @dev -- it will be the input GPIO or
>>> - * IRQ of whichever device the board code has connected up to @dev's
>>> - * output GPIO.
>>> - *
>>> - * You probably don't need to use this function -- it is used only
>>> - * by the platform-bus subsystem.
>>> - */
>>> -qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char
>>> *name, int n);
>>> -
>>>    /**
>>>     * qdev_intercept_gpio_out: Intercept an existing GPIO connection
>>>     * @dev: Device to intercept the outbound GPIO line from
>>> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
>>> index 80d07a6ec99..513ccbd1062 100644
>>> --- a/hw/core/gpio.c
>>> +++ b/hw/core/gpio.c
>>> @@ -21,6 +21,7 @@
>>>    #include "hw/qdev-core.h"
>>>    #include "hw/irq.h"
>>>    #include "qapi/error.h"
>>> +#include "qdev-internal.h"
>>>      static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
>>>                                                   const char *name)
>>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>>> index 05c1da3d311..0e6773c8df7 100644
>>> --- a/hw/core/sysbus.c
>>> +++ b/hw/core/sysbus.c
>>> @@ -23,6 +23,7 @@
>>>    #include "hw/sysbus.h"
>>>    #include "monitor/monitor.h"
>>>    #include "exec/address-spaces.h"
>>> +#include "qdev-internal.h"
>>>      static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int
>>> indent);
>>>    static char *sysbus_get_fw_dev_path(DeviceState *dev);
>> Otherwise, the tweak looks reasonable:
>> Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
> Thanks,
>
> Phil.
>
> .
diff mbox series

Patch

diff --git a/hw/core/qdev-internal.h b/hw/core/qdev-internal.h
new file mode 100644
index 00000000000..6ec17d0ea70
--- /dev/null
+++ b/hw/core/qdev-internal.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * qdev internal helpers
+ *
+ * Copyright (c) 2009-2021 QEMU contributors
+ */
+#ifndef HW_CORE_QDEV_INTERNAL_H
+#define HW_CORE_QDEV_INTERNAL_H
+
+#include "hw/qdev-core.h"
+
+/* Following functions are only used by the platform-bus subsystem */
+qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
+
+#endif /* HW_CORE_QDEV_INTERNAL_H */
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index d19c9417520..655899654bb 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -532,24 +532,6 @@  void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
                                  qemu_irq input_pin);
 
-/**
- * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
- * @dev: Device whose output GPIO we are interested in
- * @name: Name of the output GPIO array
- * @n: Number of the output GPIO line within that array
- *
- * Returns whatever qemu_irq is currently connected to the specified
- * output GPIO line of @dev. This will be NULL if the output GPIO line
- * has never been wired up to the anything.  Note that the qemu_irq
- * returned does not belong to @dev -- it will be the input GPIO or
- * IRQ of whichever device the board code has connected up to @dev's
- * output GPIO.
- *
- * You probably don't need to use this function -- it is used only
- * by the platform-bus subsystem.
- */
-qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
-
 /**
  * qdev_intercept_gpio_out: Intercept an existing GPIO connection
  * @dev: Device to intercept the outbound GPIO line from
diff --git a/hw/core/gpio.c b/hw/core/gpio.c
index 80d07a6ec99..513ccbd1062 100644
--- a/hw/core/gpio.c
+++ b/hw/core/gpio.c
@@ -21,6 +21,7 @@ 
 #include "hw/qdev-core.h"
 #include "hw/irq.h"
 #include "qapi/error.h"
+#include "qdev-internal.h"
 
 static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
                                                const char *name)
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 05c1da3d311..0e6773c8df7 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -23,6 +23,7 @@ 
 #include "hw/sysbus.h"
 #include "monitor/monitor.h"
 #include "exec/address-spaces.h"
+#include "qdev-internal.h"
 
 static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *sysbus_get_fw_dev_path(DeviceState *dev);