Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support

Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support

[Jakub Kicinski]

On Fri,  1 May 2026 10:54:19 -0500 Alex Elder wrote:
> The Toshiba TC956x is an Ethernet AVB/TSN bridge,  and is
> essentially a small and highly-specialized SoC.  It implements
> a number of internal functions, including a GPIO controller,
> control registers managing internal reset and clock control
> signals, a PCIe switch and internal endpoint, and mapping
> hardware that translates between PCIe and internal addressing.

drivers/misc/tc956x_pci.c:541:17: error: call to undeclared function 'u32_get_bits'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  541 |         chip->rev_id = u32_get_bits(val, NCID_REV_ID_MASK);
      |                        ^
-- 
pw-bot: cr

Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support

[Alex Elder]

On 5/2/26 11:45 AM, Jakub Kicinski wrote:
> On Fri,  1 May 2026 10:54:19 -0500 Alex Elder wrote:
>> The Toshiba TC956x is an Ethernet AVB/TSN bridge,  and is
>> essentially a small and highly-specialized SoC.  It implements
>> a number of internal functions, including a GPIO controller,
>> control registers managing internal reset and clock control
>> signals, a PCIe switch and internal endpoint, and mapping
>> hardware that translates between PCIe and internal addressing.
> 
> drivers/misc/tc956x_pci.c:541:17: error: call to undeclared function 'u32_get_bits'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>    541 |         chip->rev_id = u32_get_bits(val, NCID_REV_ID_MASK);
>        |                        ^

Yeah I think I noticed an error like that shows up with 32-bit builds?
In any case we didn't see it during development, and we'll make sure
<linux/bitfield.h> gets included.

Thanks.

					-Alex

Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support

[Jakub Kicinski]

On Sat, 2 May 2026 21:06:33 -0500 Alex Elder wrote:
> > drivers/misc/tc956x_pci.c:541:17: error: call to undeclared function 'u32_get_bits'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> >    541 |         chip->rev_id = u32_get_bits(val, NCID_REV_ID_MASK);
> >        |                        ^  
> 
> Yeah I think I noticed an error like that shows up with 32-bit builds?
> In any case we didn't see it during development, and we'll make sure
> <linux/bitfield.h> gets included.

on x86 it hits on all configs, I suspected you're building for ARM?

Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support

[Alex Elder]

On 5/2/26 9:14 PM, Jakub Kicinski wrote:
> On Sat, 2 May 2026 21:06:33 -0500 Alex Elder wrote:
>>> drivers/misc/tc956x_pci.c:541:17: error: call to undeclared function 'u32_get_bits'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>>>     541 |         chip->rev_id = u32_get_bits(val, NCID_REV_ID_MASK);
>>>         |                        ^
>>
>> Yeah I think I noticed an error like that shows up with 32-bit builds?
>> In any case we didn't see it during development, and we'll make sure
>> <linux/bitfield.h> gets included.
> 
> on x86 it hits on all configs, I suspected you're building for ARM?

Yes.  I'll spend a little more time trying to build on other
architectures.  I normally like to add COMPILE_TEST.

					-Alex

Re: [PATCH net-next 00/12] net: enable TC956x support

[Jakub Kicinski]

On Fri,  1 May 2026 10:54:08 -0500 Alex Elder wrote:
>  create mode 100644 Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
>  create mode 100644 drivers/gpio/gpio-tc956x.c
>  create mode 100644 drivers/misc/tc956x_pci.c
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-tc956x.c
>  create mode 100644 drivers/net/pcs/pcs-xpcs-regmap.c
>  create mode 100644 include/linux/pcs/pcs-xpcs-regmap.h
>  create mode 100644 include/soc/toshiba/tc956x-dwmac.h

Please add an entry to MAITNAINERS for tx956x stuff?

Re: [PATCH net-next 00/12] net: enable TC956x support

[Alex Elder]

On 5/2/26 11:47 AM, Jakub Kicinski wrote:
> On Fri,  1 May 2026 10:54:08 -0500 Alex Elder wrote:
>>   create mode 100644 Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
>>   create mode 100644 drivers/gpio/gpio-tc956x.c
>>   create mode 100644 drivers/misc/tc956x_pci.c
>>   create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-tc956x.c
>>   create mode 100644 drivers/net/pcs/pcs-xpcs-regmap.c
>>   create mode 100644 include/linux/pcs/pcs-xpcs-regmap.h
>>   create mode 100644 include/soc/toshiba/tc956x-dwmac.h
> 
> Please add an entry to MAITNAINERS for tx956x stuff?

OK, I'll do that for the next version of the series.  Thanks.

					-Alex

Re: [PATCH net-next 10/12] net: stmmac: tc956x: add TC956x/QPS615 support

[Mohd Ayaan Anwar]

Hi Alex,
On Fri, May 01, 2026 at 10:54:18AM -0500, Alex Elder wrote:

> +	/*
> +	 * TX956x has 8 TX queues but only #0 to #3 work for general IP traffic.

Minor typo in the comment: I think you meant TC956X instead of TX956X?

> +
> +	for (i = 0; i < td->plat->rx_queues_to_use; i++) {
> +		res->rx_irq[i] = irq_create_mapping(irq_domain, HWIRQ_RX0 + i);
> +		if (!res->tx_irq[i])

Typo: res->rx_irq instead of res->tx_irq.

PS: I was able to successfully test this series out on a Rb3Gen2 board.

	Ayaan

Re: [PATCH net-next 10/12] net: stmmac: tc956x: add TC956x/QPS615 support

[Alex Elder]

On 5/5/26 11:38 AM, Mohd Ayaan Anwar wrote:
> Hi Alex,
> On Fri, May 01, 2026 at 10:54:18AM -0500, Alex Elder wrote:
> 
>> +	/*
>> +	 * TX956x has 8 TX queues but only #0 to #3 work for general IP traffic.
> 
> Minor typo in the comment: I think you meant TC956X instead of TX956X?

Yes, I'll fix that.

>> +	for (i = 0; i < td->plat->rx_queues_to_use; i++) {
>> +		res->rx_irq[i] = irq_create_mapping(irq_domain, HWIRQ_RX0 + i);
>> +		if (!res->tx_irq[i])
> 
> Typo: res->rx_irq instead of res->tx_irq.

Wow, that's important...  Fortunately we haven't been getting errors.

This will be fixed.

> PS: I was able to successfully test this series out on a Rb3Gen2 board.

Great!  Thank you.

					-Alex

> 	Ayaan

Re: [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy

[Mohd Ayaan Anwar]

Hi Alex,
On Fri, May 01, 2026 at 10:54:20AM -0500, Alex Elder wrote:
> From: Daniel Thompson <daniel@riscstar.com>
> 
> The QCS6490 RB3Gen2 includes a Toshiba TC9564 (a.k.a. Qualcomm QPS615).
> TC9564 is an twin Ethernet-AVB/TSN bridge with an integrated PCIe switch.
> 
> There are multiple builds of RB3Gen2 with components included/excluded.
> That means whether or not there is a phy attached to eMAC0 depends on
> the exact board. However all versions include a TC9564 combined with a
> single QCS8081 attached to eMAC1.
> 
> Add properties to the existing PCI nodes to describe how the TC9564 and
> QCS8081 are connected to each other (and to the host SoC).
> 
> (Note: "pci1179,0220" is documented in the "net/toshiba,tc956x-dwmac.yaml"
> binding, but checkpatch.pl doesn't recognize that.)
> 
> Co-developed-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>

There's a minor typo in the PHY name - QCS8081 instead of QCA8081.

	Ayaan

Re: [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy

[Alex Elder]

On 5/5/26 11:42 AM, Mohd Ayaan Anwar wrote:
> Hi Alex,
> On Fri, May 01, 2026 at 10:54:20AM -0500, Alex Elder wrote:
>> From: Daniel Thompson <daniel@riscstar.com>
>>
>> The QCS6490 RB3Gen2 includes a Toshiba TC9564 (a.k.a. Qualcomm QPS615).
>> TC9564 is an twin Ethernet-AVB/TSN bridge with an integrated PCIe switch.
>>
>> There are multiple builds of RB3Gen2 with components included/excluded.
>> That means whether or not there is a phy attached to eMAC0 depends on
>> the exact board. However all versions include a TC9564 combined with a
>> single QCS8081 attached to eMAC1.
>>
>> Add properties to the existing PCI nodes to describe how the TC9564 and
>> QCS8081 are connected to each other (and to the host SoC).
>>
>> (Note: "pci1179,0220" is documented in the "net/toshiba,tc956x-dwmac.yaml"
>> binding, but checkpatch.pl doesn't recognize that.)
>>
>> Co-developed-by: Alex Elder <elder@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> 
> There's a minor typo in the PHY name - QCS8081 instead of QCA8081.

OK, I'll fix that too.  Thanks a lot Ayaan.

					-Alex

> 
> 	Ayaan

Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support

[Julian Braha]

On 5/1/26 16:54, Alex Elder wrote:
> +config GPIO_TC956X
> +	tristate "Toshiba TC956X GPIO support"
> +	depends on TOSHIBA_TC956X_PCI
> +	default m if TOSHIBA_TC956X_PCI

Hi Alex,

In your Kconfig changes, this condition 'if TOSHIBA_TC956X_PCI' is dead
code. Since you have the dependency on TOSHIBA_TC956X_PCI, you can just
make the 'default m' unconditional - assuming this is what you intended.

Perhaps you would prefer to use 'default TOSHIBA_TC956X_PCI', which
would have GPIO_TC956X default to 'm' or 'y' when TOSHIBA_TC956X_PCI is
'm' or 'y', respectively.

- Julian Braha

Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support

[Alex Elder]

On 5/2/26 10:42 PM, Julian Braha wrote:
> On 5/1/26 16:54, Alex Elder wrote:
>> +config GPIO_TC956X
>> +	tristate "Toshiba TC956X GPIO support"
>> +	depends on TOSHIBA_TC956X_PCI
>> +	default m if TOSHIBA_TC956X_PCI
> 
> Hi Alex,
> 
> In your Kconfig changes, this condition 'if TOSHIBA_TC956X_PCI' is dead
> code. Since you have the dependency on TOSHIBA_TC956X_PCI, you can just
> make the 'default m' unconditional - assuming this is what you intended.

I'm not sure I'd call it "dead" but you're right, it's not
necessary because it already depends on that symbol.

> Perhaps you would prefer to use 'default TOSHIBA_TC956X_PCI', which
> would have GPIO_TC956X default to 'm' or 'y' when TOSHIBA_TC956X_PCI is
> 'm' or 'y', respectively.

Yeah that might be better.  I'd like to eventually include
COMPILE_TEST as well, and that might need the "if" on the
default.  I'll find out whenever I test that.

This GPIO feature should still be a module even if
TOSHIBA_TC956X_PCI is y, because it's not always
necessary to enable the GPIO driver (depending on
how devicetree defines the PHY resets).

So:  In drivers/gpio/Kconfig it will be "default m", and
for drivers/net/ethernet/stmicro/stmmac/Kconfig it will be
default TOSHIBA_TC956X_PCI (at least for now).

Thanks a lot for the suggestion.

					-Alex

> - Julian Braha

Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support

[Alex Elder]

On 5/4/26 7:46 AM, Bartosz Golaszewski wrote:
> On Fri, 1 May 2026 17:54:17 +0200, Alex Elder <elder@riscstar.com> said:
>> Toshiba TC956x is an Ethernet-AVB/TSN bridge and is essentially
>> a small and highly-specialized SoC.  TC956x includes a GPIO block that
>> can be accessed, alongside several other peripherals, via two PCIe
>> endpoint functions.  The PCIe function driver creates an auxiliary
>> device for the GPIO block, and that device gets bound to this auxiliary
>> device driver.
>>
>> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>

Thanks Bartosz, I've got some responses below.

>> ---
>>   drivers/gpio/Kconfig       |  11 ++
>>   drivers/gpio/Makefile      |   1 +
>>   drivers/gpio/gpio-tc956x.c | 209 +++++++++++++++++++++++++++++++++++++
>>   3 files changed, 221 insertions(+)
>>   create mode 100644 drivers/gpio/gpio-tc956x.c
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 020e51e30317a..746cedea7e91d 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -1646,6 +1646,17 @@ config GPIO_TC3589X
>>   	  This enables support for the GPIOs found on the TC3589X
>>   	  I/O Expander.
>>
>> +config GPIO_TC956X
>> +	tristate "Toshiba TC956X GPIO support"
>> +	depends on TOSHIBA_TC956X_PCI
>> +	default m if TOSHIBA_TC956X_PCI
>> +	help
>> +	  This enables support for the GPIO controller embedded in the Toshiba
>> +	  TC956X (and Qualcomm QPS615).  This device connects to the host
>> +	  via PCIe port, which is the upstream port on an internal PCIe
>> +	  switch.  On some platforms, a few of the GPIO lines are used to
>> +	  manage external resets.
>> +
>>   config GPIO_TIMBERDALE
>>   	bool "Support for timberdale GPIO IP"
>>   	depends on MFD_TIMBERDALE
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index b267598b517de..c3584e7cba9b4 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -178,6 +178,7 @@ obj-$(CONFIG_GPIO_SYSCON)		+= gpio-syscon.o
>>   obj-$(CONFIG_GPIO_TANGIER)		+= gpio-tangier.o
>>   obj-$(CONFIG_GPIO_TB10X)		+= gpio-tb10x.o
>>   obj-$(CONFIG_GPIO_TC3589X)		+= gpio-tc3589x.o
>> +obj-$(CONFIG_GPIO_TC956X)		+= gpio-tc956x.o
>>   obj-$(CONFIG_GPIO_TEGRA186)		+= gpio-tegra186.o
>>   obj-$(CONFIG_GPIO_TEGRA)		+= gpio-tegra.o
>>   obj-$(CONFIG_GPIO_THUNDERX)		+= gpio-thunderx.o
>> diff --git a/drivers/gpio/gpio-tc956x.c b/drivers/gpio/gpio-tc956x.c
>> new file mode 100644
>> index 0000000000000..12221d8f812d9
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-tc956x.c
>> @@ -0,0 +1,209 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * Copyright (C) 2026 by RISCstar Solutions Corporation.  All rights reserved.
>> + */
>> +
>> +/*
>> + * The Toshiba TC956X implements a PCIe Gen 3 switch that connects an
>> + * upstream x4 port to two downstream PCIe x2 ports.  It incorporates
>> + * an internal endpoint on a internal PCIe port that implements two
>> + * Synopsys XGMAC Ethernet interfaces.
>> + *
>> + * 35 GPIOs are also implemented by an embedded GPIO controller.  Three
>> + * registers control the first 32 GPIOs (other than 20 and 21, which are
>> + * reserved).  Three other registers control GPIOs 32 through 36. GPIOs
>> + * 22-24, 27-28, 31, and 34 are treated as "input only".
>> + *
>> + * There is a TC956X PCI power controller driver that accesses the
>> + * direction and output value registers for GPIOs 2 and 3.  These
>> + * GPIOs control the reset signal for the two downstream PCIe ports.
>> + * Their values will never change during operation of this driver, and
>> + * this driver reserves these two GPIOS.
>> + */
>> +
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/dev_printk.h>
> 
> This is implied by device.h which is guarnteed by platform_device.h. Please
> drop it.

OK, I'll drop it.

>> +#include <linux/gpio/driver.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#define DRIVER_NAME		"tc956x-gpio"
>> +
>> +#define TC956X_GPIO_COUNT	37	/* Number of GPIOs (20-21 reserved) */
>> +
>> +/* The GPIO offsets are relative to 0x1200 in TC956X SFR space */
>> +#define GPIO_IN0_OFFSET		0x00		/* Input value (0-31) */
>> +#define GPIO_EN0_OFFSET		0x08		/* 0: out; 1: in (0-31) */
>> +#define GPIO_OUT0_OFFSET	0x10		/* Output value (0-31) */
>> +
>> +#define GPIO_IN1_OFFSET		0x04		/* Input value (32-36) */
>> +#define GPIO_EN1_OFFSET		0x0c		/* 0: out; 1: in (32-36) */
>> +#define GPIO_OUT1_OFFSET	0x14		/* Output value (32-36) */
>> +
>> +/*
>> + * struct tc956x_gpio - Information related to the embedded GPIO controller
>> + * @chip:		GPIO chip structure
>> + * @regmap:		MMIO register map for SFR GPIO region access
>> + * @input_only:		Bitmap indicating which GPIOs are input-only
>> + */
>> +struct tc956x_gpio {
>> +	struct gpio_chip chip;
>> +	struct regmap *regmap;
>> +	DECLARE_BITMAP(input_only, TC956X_GPIO_COUNT);
>> +};
>> +
>> +static int tc956x_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>> +{
>> +	struct tc956x_gpio *gpio = gpiochip_get_data(gc);
>> +	u32 reg;
>> +	u32 val;
>> +
>> +	if (test_bit(offset, gpio->input_only))
>> +		return GPIO_LINE_DIRECTION_IN;
>> +
>> +	reg = offset < 32 ? GPIO_EN0_OFFSET : GPIO_EN1_OFFSET;
>> +
>> +	regmap_read(gpio->regmap, reg, &val);
>> +	if (val & BIT(offset % 32))
>> +		return GPIO_LINE_DIRECTION_IN;
>> +
>> +	return GPIO_LINE_DIRECTION_OUT;
>> +}
>> +
>> +static int tc956x_gpio_direction_input(struct gpio_chip *gc,
>> +				       unsigned int offset)
>> +{
>> +	u32 reg = offset < 32 ? GPIO_EN0_OFFSET : GPIO_EN1_OFFSET;
>> +	struct tc956x_gpio *gpio = gpiochip_get_data(gc);
>> +	u32 mask = BIT(offset % 32);
>> +
>> +	return regmap_update_bits(gpio->regmap, reg, mask, mask);
>> +}
>> +
>> +static int tc956x_gpio_direction_output(struct gpio_chip *gc,
>> +					unsigned int offset, int value)
>> +{
>> +	struct tc956x_gpio *gpio = gpiochip_get_data(gc);
>> +	u32 vreg;
>> +	u32 dreg;
>> +	u32 mask;
>> +
>> +	if (test_bit(offset, gpio->input_only))
>> +		return -EINVAL;
>> +
>> +	if (offset < 32) {
>> +		vreg = GPIO_OUT0_OFFSET;
>> +		dreg = GPIO_EN0_OFFSET;
>> +	} else {
>> +		vreg = GPIO_OUT1_OFFSET;
>> +		dreg = GPIO_EN1_OFFSET;
>> +	}
>> +	mask = BIT(offset % 32);
>> +
>> +	/* Set output value first, then direction */
>> +	regmap_update_bits(gpio->regmap, vreg, mask, value ? mask : 0);
>> +
>> +	return regmap_update_bits(gpio->regmap, dreg, mask, 0);
>> +}
>> +
>> +static int tc956x_gpio_get(struct gpio_chip *gc, unsigned int offset)
>> +{
>> +	u32 reg = offset < 32 ? GPIO_IN0_OFFSET : GPIO_IN1_OFFSET;
>> +	struct tc956x_gpio *gpio = gpiochip_get_data(gc);
>> +	u32 val;
>> +
>> +	regmap_read(gpio->regmap, reg, &val);
>> +
>> +	return val & BIT(offset % 32) ? 1 : 0;
>> +}
>> +
>> +static int tc956x_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
>> +{
>> +	u32 reg = offset < 32 ? GPIO_OUT0_OFFSET : GPIO_OUT1_OFFSET;
>> +	struct tc956x_gpio *gpio = gpiochip_get_data(gc);
>> +	u32 mask = BIT(offset % 32);
>> +
>> +	return regmap_update_bits(gpio->regmap, reg, mask, value ? mask : 0);
>> +}
>> +
>> +static int tc956x_gpio_init_valid_mask(struct gpio_chip *gc,
>> +				       unsigned long *valid_mask,
>> +				       unsigned int ngpios)
>> +{
>> +	/*
>> +	 * GPIOs 2 and 3 are used by the PCI power control driver, and
>> +	 * we don't allow them to be used.  GPIOs 20 and 21 are reserved
>> +	 * (and not usable).
>> +	 */
>> +	bitmap_fill(valid_mask, ngpios);
>> +	bitmap_clear(valid_mask, 2, 2);
>> +	bitmap_clear(valid_mask, 20, 2);
>> +
>> +	return 0;
>> +}
>> +
>> +static int tc956x_gpio_probe(struct auxiliary_device *adev,
>> +			     const struct auxiliary_device_id *id)
>> +{
>> +	struct device *dev = &adev->dev;
>> +	struct tc956x_gpio *gpio;
>> +	struct gpio_chip *gc;
>> +
>> +	if (!dev->platform_data)
>> +		return -EINVAL;
>> +
>> +	gpio = devm_kzalloc(dev, sizeof(*gpio), GFP_KERNEL);
>> +	if (!gpio)
>> +		return -ENOMEM;
> 
> Add newline.

I will add a newline here.

> 
>> +	gpio->regmap = dev->platform_data;
> 
> It's not clear whether this is an mmio regmap or a slow-bus one that can fail.
> In the code above you're checking the return values of regmap operations quite
> inconsistently. Could you please verify if you need it and either always check
> them or not at all?

You're right.  I'm returning the result of regmap_update_bits()
from two callback functions.  But yes, this is an MMIO regmap.

I will return 0 and ignore the return value of regmap_update_bits()
in those spots, and will add some comments that make it clear that
this is an MMIO regmap.

>> +
>> +	/* Mark GPIOs 22, 23, 24, 27, 28, 31, and 34 as input only */
>> +	bitmap_set(gpio->input_only, 22, 3);
>> +	bitmap_set(gpio->input_only, 27, 2);
>> +	set_bit(31, gpio->input_only);
>> +	set_bit(34, gpio->input_only);
>> +
>> +	gc = &gpio->chip;
>> +
>> +	gc->label = DRIVER_NAME;
>> +	gc->parent = dev->parent;
>> +
>> +	gc->get_direction = tc956x_gpio_get_direction;
>> +	gc->direction_input = tc956x_gpio_direction_input;
>> +	gc->direction_output = tc956x_gpio_direction_output;
>> +	gc->get = tc956x_gpio_get;
>> +	gc->set = tc956x_gpio_set;
>> +	gc->init_valid_mask = tc956x_gpio_init_valid_mask;
>> +
>> +	gc->base = -1;
>> +	gc->ngpio = TC956X_GPIO_COUNT;
>> +	gc->can_sleep = false;
> 
> This makes me think this is an MMIO regmap after all.

You are correct.

> 
>> +
>> +	dev_set_drvdata(dev, gpio);
> 
> There's no corresponding dev_get_drvdata().

I hadn't noticed that.  We're only using the GPIO chip data
field (gc->gpiodev->data) instead.  I'll drop this call.

>> +
>> +	return devm_gpiochip_add_data(dev, gc, gpio);
>> +}
>> +
>> +static const struct auxiliary_device_id tc956x_gpio_ids[] = {
>> +	{ .name = "tc956x_pci.tc9564-gpio", },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(auxiliary, tc956x_gpio_ids);
>> +
>> +static struct auxiliary_driver tc956x_gpio_driver = {
>> +	.name		= DRIVER_NAME,
>> +	.probe          = tc956x_gpio_probe,
>> +	.id_table       = tc956x_gpio_ids,
>> +	.driver = {
>> +		.name		= DRIVER_NAME,
>> +		.owner		= THIS_MODULE,
>> +		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,
>> +	},
>> +};
>> +module_auxiliary_driver(tc956x_gpio_driver);
>> +
>> +MODULE_DESCRIPTION("Toshiba TC956X PCIe GPIO Driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("auxiliary:" DRIVER_NAME);
>> --
>> 2.51.0
>>
>>
> 
> There are a few minor issues but overall looks good!

Thank you very much for the review Bartosz.  I'll implement
all of your suggestions in the next version.

					-Alex


> 
> Bart

Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support

[Alex Elder]

On 5/6/26 4:43 PM, Andrew Lunn wrote:
>> To be clear, the reason you're asking is that you're suggesting
>> we might want to model the GPIO controller differently, correct?
> 
> Correct.
> 
>> I.e., model it as *not* associated with the embedded PCIe
>> functions.  Then we need to think about what its parent device
>> would be (the power control device, which I think somehow
>> duplicates the switch device?).
> 
> Logically, the GPIO controller cannot be part of a downstream
> function, if you need to use the GPIO controller to turn the
> downstream function on.

Yes you're right, though the PCIe power controller functions
before the PCIe switch is enabled, and uses I2C to communicate
with the host.

> Logically, the GPIO controller needs to be above the switch downstream
> end points. Where above, i don't know. Which is why i was asking about
> where it appears in the address spaces.

You are touching on an issue we have faced since we started on
this earlier in the year.  Our objective was to enable the eMACs,
but there was no device representing the "chip" (which holds the
switch and the GPIO controller, etc.).  The TC956x is more than
just a PCIe switch, and more than just two Ethernet MACs.  The
vendor code handles some of this between PCIe functions with
some reference counting and perhaps other things.

Eventually we settled on the model we have presented, which
creates a device for each function and lets one of them take
care of common "chip" things (including creating the GPIO
auxiliary device).  The function device driver creates a
new auxiliary device to represent the MAC for each function.

I also considered modeling the TC956x as a remoteproc, but have
been reluctant to really pursue that.

> But i also don't know too much about PCI, i'm used to SoCs with simple
> linear MMIO.

I'm no PCI expert either, but I'm learning.

>  From the little i know, it is more than what address space does the
> GPIO appear in. Its also, what enumerable entity does it appear in
> within the PCI bus. Because its the enumeration which is going to
> trigger a driver load, which can then drive the GPIO controller.
> 
> Or, something more radical, you make the PCIe power controller an MFD,
> instantiating both the power controller and a GPIO controller over the
> I2C bus. GPIO access will not be as fast, but is there anything here
> which needs to be fast?

I considered that, but opted not to mess with the PCIe power controller
driver.

It's only asserting resets in the RB3gen2, so I don't the speed is a
major factor.

					-Alex


> 
>        Andrew

Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support

[Linus Walleij]

Hi Alex,

thanks for your patch!

On Fri, May 1, 2026 at 5:55 PM Alex Elder <elder@riscstar.com> wrote:

> Toshiba TC956x is an Ethernet-AVB/TSN bridge and is essentially
> a small and highly-specialized SoC.  TC956x includes a GPIO block that
> can be accessed, alongside several other peripherals, via two PCIe
> endpoint functions.  The PCIe function driver creates an auxiliary
> device for the GPIO block, and that device gets bound to this auxiliary
> device driver.
>
> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
(...)

> +config GPIO_TC956X
> +       tristate "Toshiba TC956X GPIO support"
> +       depends on TOSHIBA_TC956X_PCI
> +       default m if TOSHIBA_TC956X_PCI

I think this driver can

select GPIO_REGMAP

> +#include <linux/auxiliary_bus.h>
> +#include <linux/dev_printk.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>

#include <linux/gpio/regmap.h>

> +#define TC956X_GPIO_COUNT      37      /* Number of GPIOs (20-21 reserved) */

I would just do 64 and flag > 37 as invalid.

> +/*
> + * struct tc956x_gpio - Information related to the embedded GPIO controller
> + * @chip:              GPIO chip structure
> + * @regmap:            MMIO register map for SFR GPIO region access
> + * @input_only:                Bitmap indicating which GPIOs are input-only
> + */
> +struct tc956x_gpio {
> +       struct gpio_chip chip;
> +       struct regmap *regmap;
> +       DECLARE_BITMAP(input_only, TC956X_GPIO_COUNT);

> +static int tc956x_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
> +static int tc956x_gpio_direction_input(struct gpio_chip *gc,
> +                                      unsigned int offset)
> +static int tc956x_gpio_direction_output(struct gpio_chip *gc,
> +                                       unsigned int offset, int value)
> +static int tc956x_gpio_get(struct gpio_chip *gc, unsigned int offset)
> +static int tc956x_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)

REGMAP_GPIO can handle all of this for you with the right
parameterization, study the drivers using this already such as
those that appear when you type
git grep 'gpio\/regmap\.h'


> +static int tc956x_gpio_init_valid_mask(struct gpio_chip *gc,
> +                                      unsigned long *valid_mask,
> +                                      unsigned int ngpios)
> +{
> +       /*
> +        * GPIOs 2 and 3 are used by the PCI power control driver, and
> +        * we don't allow them to be used.  GPIOs 20 and 21 are reserved
> +        * (and not usable).
> +        */
> +       bitmap_fill(valid_mask, ngpios);
> +       bitmap_clear(valid_mask, 2, 2);
> +       bitmap_clear(valid_mask, 20, 2);
> +
> +       return 0;
> +}

That's good use of this facility.

I would say the chip has 64 lines and just
bitmap_clear(valid_mask, 37, 64 - 37);
but that's your pick. This probably works too.

> +       /* Mark GPIOs 22, 23, 24, 27, 28, 31, and 34 as input only */
> +       bitmap_set(gpio->input_only, 22, 3);
> +       bitmap_set(gpio->input_only, 27, 2);
> +       set_bit(31, gpio->input_only);
> +       set_bit(34, gpio->input_only);

regmap-gpio can't currently handle selective input-only or
output-only lines, but we can
very easily make it.

So I sent a patch for that (now in your inbox).

Check if this fixed_direction_sparse bitmap will do the trick
for you and provide Tested-by if it does, thanks!

Yours,
Linus Walleij

Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support

[Alex Elder]

On 5/7/26 7:15 AM, Linus Walleij wrote:
> Hi Alex,
> 
> thanks for your patch!

Thank you for your excellent feedback.  I will plan to use
regmap-gpio (already suggested strongly by Andrew Lunn) and
that will be included in the next version of the series.
Once I've done this and tried your other patch I'll provide
a Tested-by for it.

					-Alex


> 
> On Fri, May 1, 2026 at 5:55 PM Alex Elder <elder@riscstar.com> wrote:
> 
>> Toshiba TC956x is an Ethernet-AVB/TSN bridge and is essentially
>> a small and highly-specialized SoC.  TC956x includes a GPIO block that
>> can be accessed, alongside several other peripherals, via two PCIe
>> endpoint functions.  The PCIe function driver creates an auxiliary
>> device for the GPIO block, and that device gets bound to this auxiliary
>> device driver.
>>
>> Co-developed-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
> (...)
> 
>> +config GPIO_TC956X
>> +       tristate "Toshiba TC956X GPIO support"
>> +       depends on TOSHIBA_TC956X_PCI
>> +       default m if TOSHIBA_TC956X_PCI
> 
> I think this driver can
> 
> select GPIO_REGMAP
> 
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/dev_printk.h>
>> +#include <linux/gpio/driver.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
> 
> #include <linux/gpio/regmap.h>
> 
>> +#define TC956X_GPIO_COUNT      37      /* Number of GPIOs (20-21 reserved) */
> 
> I would just do 64 and flag > 37 as invalid.
> 
>> +/*
>> + * struct tc956x_gpio - Information related to the embedded GPIO controller
>> + * @chip:              GPIO chip structure
>> + * @regmap:            MMIO register map for SFR GPIO region access
>> + * @input_only:                Bitmap indicating which GPIOs are input-only
>> + */
>> +struct tc956x_gpio {
>> +       struct gpio_chip chip;
>> +       struct regmap *regmap;
>> +       DECLARE_BITMAP(input_only, TC956X_GPIO_COUNT);
> 
>> +static int tc956x_gpio_get_direction(struct gpio_chip *gc, unsigned int offset)
>> +static int tc956x_gpio_direction_input(struct gpio_chip *gc,
>> +                                      unsigned int offset)
>> +static int tc956x_gpio_direction_output(struct gpio_chip *gc,
>> +                                       unsigned int offset, int value)
>> +static int tc956x_gpio_get(struct gpio_chip *gc, unsigned int offset)
>> +static int tc956x_gpio_set(struct gpio_chip *gc, unsigned int offset, int value)
> 
> REGMAP_GPIO can handle all of this for you with the right
> parameterization, study the drivers using this already such as
> those that appear when you type
> git grep 'gpio\/regmap\.h'
> 
> 
>> +static int tc956x_gpio_init_valid_mask(struct gpio_chip *gc,
>> +                                      unsigned long *valid_mask,
>> +                                      unsigned int ngpios)
>> +{
>> +       /*
>> +        * GPIOs 2 and 3 are used by the PCI power control driver, and
>> +        * we don't allow them to be used.  GPIOs 20 and 21 are reserved
>> +        * (and not usable).
>> +        */
>> +       bitmap_fill(valid_mask, ngpios);
>> +       bitmap_clear(valid_mask, 2, 2);
>> +       bitmap_clear(valid_mask, 20, 2);
>> +
>> +       return 0;
>> +}
> 
> That's good use of this facility.
> 
> I would say the chip has 64 lines and just
> bitmap_clear(valid_mask, 37, 64 - 37);
> but that's your pick. This probably works too.
> 
>> +       /* Mark GPIOs 22, 23, 24, 27, 28, 31, and 34 as input only */
>> +       bitmap_set(gpio->input_only, 22, 3);
>> +       bitmap_set(gpio->input_only, 27, 2);
>> +       set_bit(31, gpio->input_only);
>> +       set_bit(34, gpio->input_only);
> 
> regmap-gpio can't currently handle selective input-only or
> output-only lines, but we can
> very easily make it.
> 
> So I sent a patch for that (now in your inbox).
> 
> Check if this fixed_direction_sparse bitmap will do the trick
> for you and provide Tested-by if it does, thanks!
> 
> Yours,
> Linus Walleij

Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge

[Bjorn Andersson]

On Fri, May 01, 2026 at 10:54:16AM -0500, Alex Elder wrote:
> diff --git a/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
[..]
> +
> +  gpio-controller: true

I don't have any concern with the use of a proper gpio driver to model
the implementation, but if I understand correctly this relationship
between gpio controller and gpio consumer is strictly internal to "the
PCI device".

Is this connection variable or is the link merely expressed in
DeviceTree to mitigate the fact that you choose to implement the
responsibilities of the two parts split into two device drivers?

Are there other consumers of these TC956x gpios which would result in a
board designer (and hence dts author) to ever reference this
gpio-controller in a different way?

Regards,
Bjorn

Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge

[Alex Elder]

On 5/7/26 9:12 AM, Bjorn Andersson wrote:
> On Fri, May 01, 2026 at 10:54:16AM -0500, Alex Elder wrote:
>> diff --git a/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
> [..]
>> +
>> +  gpio-controller: true
> 
> I don't have any concern with the use of a proper gpio driver to model
> the implementation, but if I understand correctly this relationship
> between gpio controller and gpio consumer is strictly internal to "the
> PCI device".

(I think you're already cool with this but I still wanted to respond.)

That is not correct.  These GPIO lines are used two ways for the
RB3gen2:
- drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c uses GPIOs 2 and 3 to
   assert/deassert the reset lines associated with the two exposed
   downstream PCIe ports on the PCIe switch within the TC956x.

- Each of the Ethernet PHYs has a reset GPIO.  On the RB3gen2, the
   GPIOs used for the purpose come from the GPIO controller embedded
   in the TC9564 (00 and 01).

These are therefore "exposed" (they are *not* strictly internal).

> Is this connection variable or is the link merely expressed in
> DeviceTree to mitigate the fact that you choose to implement the
> responsibilities of the two parts split into two device drivers?

It is variable.  These resets might be implemented by other GPIO
controllers on other platforms.

> Are there other consumers of these TC956x gpios which would result in a
> board designer (and hence dts author) to ever reference this
> gpio-controller in a different way?

They could.  Nine of these GPIOs are exposed by the TC956x pins
(GPIO00-06, GPIO12, GPIO35 and GPIO36).  The RB3gen2 uses 00-03
(and possibly 04 but that's for a PHY we haven't tested yet).

					-Alex

> Regards,
> Bjorn

Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge

[Bjorn Andersson]

On Thu, May 07, 2026 at 01:37:09PM -0500, Alex Elder wrote:
> On 5/7/26 9:12 AM, Bjorn Andersson wrote:
> > On Fri, May 01, 2026 at 10:54:16AM -0500, Alex Elder wrote:
> > > diff --git a/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
> > [..]
> > > +
> > > +  gpio-controller: true
> > 
> > I don't have any concern with the use of a proper gpio driver to model
> > the implementation, but if I understand correctly this relationship
> > between gpio controller and gpio consumer is strictly internal to "the
> > PCI device".
> 
> (I think you're already cool with this but I still wanted to respond.)
> 

Thank you for the further clarifications, and added details, Alex. This
does look reasonable to me.

Regards,
Bjorn

> That is not correct.  These GPIO lines are used two ways for the
> RB3gen2:
> - drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c uses GPIOs 2 and 3 to
>   assert/deassert the reset lines associated with the two exposed
>   downstream PCIe ports on the PCIe switch within the TC956x.
> 
> - Each of the Ethernet PHYs has a reset GPIO.  On the RB3gen2, the
>   GPIOs used for the purpose come from the GPIO controller embedded
>   in the TC9564 (00 and 01).
> 
> These are therefore "exposed" (they are *not* strictly internal).
> 
> > Is this connection variable or is the link merely expressed in
> > DeviceTree to mitigate the fact that you choose to implement the
> > responsibilities of the two parts split into two device drivers?
> 
> It is variable.  These resets might be implemented by other GPIO
> controllers on other platforms.
> 
> > Are there other consumers of these TC956x gpios which would result in a
> > board designer (and hence dts author) to ever reference this
> > gpio-controller in a different way?
> 
> They could.  Nine of these GPIOs are exposed by the TC956x pins
> (GPIO00-06, GPIO12, GPIO35 and GPIO36).  The RB3gen2 uses 00-03
> (and possibly 04 but that's for a PHY we haven't tested yet).
> 
> 					-Alex
> 
> > Regards,
> > Bjorn
> 

Re: [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge

[Rob Herring]

On Fri, May 1, 2026 at 10:55 AM Alex Elder <elder@riscstar.com> wrote:
>
> From: Daniel Thompson <daniel@riscstar.com>
>
> Add devicetree bindings for the Toshiba TC956x family of Ethernet-AVB/TSN
> bridges.
>
> Signed-off-by: Daniel Thompson <daniel@riscstar.com>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
>  .../bindings/net/toshiba,tc956x-dwmac.yaml    | 111 ++++++++++++++++++
>  1 file changed, 111 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
> new file mode 100644
> index 0000000000000..d95d22a3761da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/toshiba,tc956x-dwmac.yaml
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/toshiba,tc956x-dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba TC956x Ethernet-AVB/TSN Controller
> +
> +maintainers:
> +  - Alex Elder <elder@riscstar.com>
> +  - Daniel Thompson <daniel@riscstar.com>
> +
> +description: |
> +  This node provides properties for configuring the Ethernet PCI functions
> +  that are attached to the internal downstream port of the TC956x's PCIe
> +  switch.
> +
> +  TC956x are a family of Ethernet-AVB/TSN bridge chips that combine a PCIe
> +  switch together with a number of Ethernet controllers. These bindings
> +  cover only the Ethernet functions of these devices.
> +
> +allOf:
> +  - $ref: /schemas/pci/pci-bus-common.yaml#
> +  - $ref: /schemas/pci/pci-device.yaml#
> +
> +unevaluatedProperties: false
> +
> +properties:
> +  compatible:
> +    enum:
> +      - pci1179,0220 # Toshiba TC9564 (a.k.a. Qualcomm QPS615)
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  # We can't allOf reference Ethernet-controller.yaml because we end up with
> +  # contradictory $nodename rules (`ethernet@` versus `pci@`). Happily only a
> +  # small number of the properties are useful on TC956x so we can just reference
> +  # what we need.

That would be due to the error sashiko pointed out. 'pci' is for PCI
bridges (host or PCI-PCI).

Rob