While at it and since they are user-creatable, build them when
CONFIG_I2C_DEVICES is set.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/gpio/Kconfig | 10 ++++++++++
 hw/misc/Kconfig |  8 --------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index c423e10f59..007048c688 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -16,6 +16,16 @@ config SIFIVE_GPIO
 config STM32L4X5_GPIO
     bool
 
+config PCA9552
+    bool
+    depends on I2C
+    default y if I2C_DEVICES
+
+config PCA9554
+    bool
+    depends on I2C
+    default y if I2C_DEVICES
+
 config PCF8574
     bool
     depends on I2C
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index 8f9ce2f68c..4271e2f4ac 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -30,14 +30,6 @@ config EDU
     default y if TEST_DEVICES
     depends on PCI && MSI_NONBROKEN
 
-config PCA9552
-    bool
-    depends on I2C
-
-config PCA9554
-    bool
-    depends on I2C
-
 config I2C_ECHO
     bool
     default y if TEST_DEVICES
-- 
2.48.1

On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow <shentey@gmail.com> wrote:
>
> While at it and since they are user-creatable, build them when
> CONFIG_I2C_DEVICES is set.

The patch subject says this is just a rearrangement
of the Kconfig stanzas with no behaviour change, but then the
commit message body includes one.

If you want to build these when CONFIG_I2C_DEVICES is set,
that should be its own patch that does that.

(The move of the Kconfig bits to hw/gpio is fixing a bug
in 6328d8ffa6cb9d ("misc/pca955*: Move models under hw/gpio"),
which moved the code but forgot to move the Kconfig sections.)

Separately, it's unclear to me how worthwhile it is to add
these to CONFIG_I2C_DEVICES, because they're GPIO devices,
which means there's not much you can do with them as a user:
as far as I know you can't wire the output/input GPIO lines
up to anything. We have the device models mainly for boards
which provide them, so that guest code that expects to see
them doesn't fall over on bootup, and because the board
model code does have the APIs to wire up the GPIO lines.

thanks
-- PMM

Am 17. Februar 2025 13:35:04 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>> While at it and since they are user-creatable, build them when
>> CONFIG_I2C_DEVICES is set.
>
>The patch subject says this is just a rearrangement
>of the Kconfig stanzas with no behaviour change, but then the
>commit message body includes one.
>
>If you want to build these when CONFIG_I2C_DEVICES is set,
>that should be its own patch that does that.
>
>(The move of the Kconfig bits to hw/gpio is fixing a bug
>in 6328d8ffa6cb9d ("misc/pca955*: Move models under hw/gpio"),
>which moved the code but forgot to move the Kconfig sections.)

Okay, I'll split the patch and use above commit message.

>
>Separately, it's unclear to me how worthwhile it is to add
>these to CONFIG_I2C_DEVICES, because they're GPIO devices,
>which means there's not much you can do with them as a user:
>as far as I know you can't wire the output/input GPIO lines
>up to anything. We have the device models mainly for boards
>which provide them, so that guest code that expects to see
>them doesn't fall over on bootup, and because the board
>model code does have the APIs to wire up the GPIO lines.

It's basically to satisfy Linux which will clog the i2c bus if such a GPIO expander is configured in the DTB but missing in the emulation (it will defer probing which will never make progress). Once it is its own patch we can decide separately how to proceed with it, e.g. dropping.

Best regards,
Bernhard

>
>thanks
>-- PMM

On Mon, 17 Feb 2025 at 20:21, Bernhard Beschow <shentey@gmail.com> wrote:
>
>
>
> Am 17. Februar 2025 13:35:04 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
> >On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow <shentey@gmail.com> wrote:
> >>
> >> While at it and since they are user-creatable, build them when
> >> CONFIG_I2C_DEVICES is set.
> >
> >The patch subject says this is just a rearrangement
> >of the Kconfig stanzas with no behaviour change, but then the
> >commit message body includes one.
> >
> >If you want to build these when CONFIG_I2C_DEVICES is set,
> >that should be its own patch that does that.
> >
> >(The move of the Kconfig bits to hw/gpio is fixing a bug
> >in 6328d8ffa6cb9d ("misc/pca955*: Move models under hw/gpio"),
> >which moved the code but forgot to move the Kconfig sections.)
>
> Okay, I'll split the patch and use above commit message.
>
> >
> >Separately, it's unclear to me how worthwhile it is to add
> >these to CONFIG_I2C_DEVICES, because they're GPIO devices,
> >which means there's not much you can do with them as a user:
> >as far as I know you can't wire the output/input GPIO lines
> >up to anything. We have the device models mainly for boards
> >which provide them, so that guest code that expects to see
> >them doesn't fall over on bootup, and because the board
> >model code does have the APIs to wire up the GPIO lines.
>
> It's basically to satisfy Linux which will clog the i2c bus if such a GPIO expander is configured in the DTB but missing in the emulation (it will defer probing which will never make progress). Once it is its own patch we can decide separately how to proceed with it, e.g. dropping.

If Linux wants to see it because it's in the dtb for the
hardware it sounds like the right answer is that we
should create it in the board code, which we can do
without adding it to CONFIG_I2C_DEVICES, because we
can make the board code's Kconfig do "select PCA9552",
like the aspeed board does already.

thanks
-- PMM

Am 18. Februar 2025 10:33:26 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>On Mon, 17 Feb 2025 at 20:21, Bernhard Beschow <shentey@gmail.com> wrote:
>>
>>
>>
>> Am 17. Februar 2025 13:35:04 UTC schrieb Peter Maydell <peter.maydell@linaro.org>:
>> >On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow <shentey@gmail.com> wrote:
>> >>
>> >> While at it and since they are user-creatable, build them when
>> >> CONFIG_I2C_DEVICES is set.
>> >
>> >The patch subject says this is just a rearrangement
>> >of the Kconfig stanzas with no behaviour change, but then the
>> >commit message body includes one.
>> >
>> >If you want to build these when CONFIG_I2C_DEVICES is set,
>> >that should be its own patch that does that.
>> >
>> >(The move of the Kconfig bits to hw/gpio is fixing a bug
>> >in 6328d8ffa6cb9d ("misc/pca955*: Move models under hw/gpio"),
>> >which moved the code but forgot to move the Kconfig sections.)
>>
>> Okay, I'll split the patch and use above commit message.
>>
>> >
>> >Separately, it's unclear to me how worthwhile it is to add
>> >these to CONFIG_I2C_DEVICES, because they're GPIO devices,
>> >which means there's not much you can do with them as a user:
>> >as far as I know you can't wire the output/input GPIO lines
>> >up to anything. We have the device models mainly for boards
>> >which provide them, so that guest code that expects to see
>> >them doesn't fall over on bootup, and because the board
>> >model code does have the APIs to wire up the GPIO lines.
>>
>> It's basically to satisfy Linux which will clog the i2c bus if such a GPIO expander is configured in the DTB but missing in the emulation (it will defer probing which will never make progress). Once it is its own patch we can decide separately how to proceed with it, e.g. dropping.
>
>If Linux wants to see it because it's in the dtb for the
>hardware it sounds like the right answer is that we
>should create it in the board code, which we can do
>without adding it to CONFIG_I2C_DEVICES, because we
>can make the board code's Kconfig do "select PCA9552",
>like the aspeed board does already.

These devices are primarily intended for modeling our custom hardware on the command line, for the purpose explained in [1]. While the real imx8mp-evk has a tca6416, I'd rather avoid creating it in board code, even if there was a model for it in QEMU. The reason is that the machine works fine without it as is, and that omitting hardcoded peripherals seems to increase the utility of the machine because it allows users to customize their machines without hardcoded peripherals getting into their way.

Since the two device models in this patch work by chance if other machines select them I'm fine with not implying CONFIG_I2C_DEVICES.

Best regards,
Bernhard

[1] https://lore.kernel.org/qemu-devel/831901E4-69B2-4637-8507-77C7BF4DA65D@gmail.com/

>
>thanks
>-- PMM