public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support
       [not found] ` <20260501155421.3329862-12-elder@riscstar.com>
@ 2026-05-02 16:45   ` Jakub Kicinski
  2026-05-03  2:06     ` Alex Elder
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2026-05-02 16:45 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, daniel, mohd.anwar, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 00/12] net: enable TC956x support
       [not found] <20260501155421.3329862-1-elder@riscstar.com>
       [not found] ` <20260501155421.3329862-12-elder@riscstar.com>
@ 2026-05-02 16:47 ` Jakub Kicinski
  2026-05-03  2:07   ` Alex Elder
       [not found] ` <20260501155421.3329862-10-elder@riscstar.com>
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2026-05-02 16:47 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, daniel, mohd.anwar, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support
  2026-05-02 16:45   ` [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support Jakub Kicinski
@ 2026-05-03  2:06     ` Alex Elder
  2026-05-03  2:14       ` Jakub Kicinski
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2026-05-03  2:06 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, daniel, mohd.anwar, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 00/12] net: enable TC956x support
  2026-05-02 16:47 ` [PATCH net-next 00/12] net: enable TC956x support Jakub Kicinski
@ 2026-05-03  2:07   ` Alex Elder
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2026-05-03  2:07 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, daniel, mohd.anwar, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support
  2026-05-03  2:06     ` Alex Elder
@ 2026-05-03  2:14       ` Jakub Kicinski
  2026-05-03  2:23         ` Alex Elder
  0 siblings, 1 reply; 12+ messages in thread
From: Jakub Kicinski @ 2026-05-03  2:14 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, daniel, mohd.anwar, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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?


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support
  2026-05-03  2:14       ` Jakub Kicinski
@ 2026-05-03  2:23         ` Alex Elder
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2026-05-03  2:23 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: andrew+netdev, davem, edumazet, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, daniel, mohd.anwar, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support
       [not found] ` <20260501155421.3329862-10-elder@riscstar.com>
@ 2026-05-03  3:42   ` Julian Braha
       [not found]   ` <CAMRc=McWXCqyv1LmWMuEMmE3HqaURx_eMD8rkDs9AJT+7W2aYw@mail.gmail.com>
  1 sibling, 0 replies; 12+ messages in thread
From: Julian Braha @ 2026-05-03  3:42 UTC (permalink / raw)
  To: Alex Elder, andrew+netdev, davem, edumazet, kuba, pabeni,
	maxime.chevallier, rmk+kernel, andersson, konradybcio, robh,
	krzk+dt, conor+dt, linusw, brgl, arnd, gregkh
  Cc: daniel, mohd.anwar, a0987203069, alexandre.torgue, ast,
	boon.khai.ng, chenchuangyu, chenhuacai, daniel, hawk, hkallweit1,
	inochiama, john.fastabend, livelycarpet87, matthew.gerlach,
	mcoquelin.stm32, me, prabhakar.mahadev-lad.rj, richardcochran,
	rohan.g.thomas, sdf, siyanteng, weishangjuan, wens, netdev, bpf,
	linux-arm-msm, devicetree, linux-gpio, linux-stm32,
	linux-arm-kernel, linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support
       [not found]   ` <CAMRc=McWXCqyv1LmWMuEMmE3HqaURx_eMD8rkDs9AJT+7W2aYw@mail.gmail.com>
@ 2026-05-04 13:07     ` Alex Elder
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2026-05-04 13:07 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: daniel, mohd.anwar, a0987203069, alexandre.torgue, ast,
	boon.khai.ng, chenchuangyu, chenhuacai, daniel, hawk, hkallweit1,
	inochiama, john.fastabend, julianbraha, livelycarpet87,
	matthew.gerlach, mcoquelin.stm32, me, prabhakar.mahadev-lad.rj,
	richardcochran, rohan.g.thomas, sdf, siyanteng, weishangjuan,
	wens, netdev, bpf, linux-arm-msm, devicetree, linux-gpio,
	linux-stm32, linux-arm-kernel, linux-kernel, andrew+netdev, davem,
	edumazet, kuba, pabeni, maxime.chevallier, rmk+kernel, andersson,
	konradybcio, robh, krzk+dt, conor+dt, linusw, arnd, gregkh

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 10/12] net: stmmac: tc956x: add TC956x/QPS615 support
       [not found] ` <20260501155421.3329862-11-elder@riscstar.com>
@ 2026-05-05 16:38   ` Mohd Ayaan Anwar
  2026-05-05 16:46     ` Alex Elder
  0 siblings, 1 reply; 12+ messages in thread
From: Mohd Ayaan Anwar @ 2026-05-05 16:38 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, Daniel Thompson, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy
       [not found] ` <20260501155421.3329862-13-elder@riscstar.com>
@ 2026-05-05 16:42   ` Mohd Ayaan Anwar
  2026-05-05 16:46     ` Alex Elder
  0 siblings, 1 reply; 12+ messages in thread
From: Mohd Ayaan Anwar @ 2026-05-05 16:42 UTC (permalink / raw)
  To: Alex Elder
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, Daniel Thompson, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 10/12] net: stmmac: tc956x: add TC956x/QPS615 support
  2026-05-05 16:38   ` [PATCH net-next 10/12] net: stmmac: " Mohd Ayaan Anwar
@ 2026-05-05 16:46     ` Alex Elder
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2026-05-05 16:46 UTC (permalink / raw)
  To: Mohd Ayaan Anwar
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, Daniel Thompson, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy
  2026-05-05 16:42   ` [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy Mohd Ayaan Anwar
@ 2026-05-05 16:46     ` Alex Elder
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Elder @ 2026-05-05 16:46 UTC (permalink / raw)
  To: Mohd Ayaan Anwar
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, maxime.chevallier,
	rmk+kernel, andersson, konradybcio, robh, krzk+dt, conor+dt,
	linusw, brgl, arnd, gregkh, Daniel Thompson, a0987203069,
	alexandre.torgue, ast, boon.khai.ng, chenchuangyu, chenhuacai,
	daniel, hawk, hkallweit1, inochiama, john.fastabend, julianbraha,
	livelycarpet87, matthew.gerlach, mcoquelin.stm32, me,
	prabhakar.mahadev-lad.rj, richardcochran, rohan.g.thomas, sdf,
	siyanteng, weishangjuan, wens, netdev, bpf, linux-arm-msm,
	devicetree, linux-gpio, linux-stm32, linux-arm-kernel,
	linux-kernel

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-05-05 16:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260501155421.3329862-1-elder@riscstar.com>
     [not found] ` <20260501155421.3329862-12-elder@riscstar.com>
2026-05-02 16:45   ` [PATCH net-next 11/12] misc: tc956x_pci: add TC956x/QPS615 support Jakub Kicinski
2026-05-03  2:06     ` Alex Elder
2026-05-03  2:14       ` Jakub Kicinski
2026-05-03  2:23         ` Alex Elder
2026-05-02 16:47 ` [PATCH net-next 00/12] net: enable TC956x support Jakub Kicinski
2026-05-03  2:07   ` Alex Elder
     [not found] ` <20260501155421.3329862-10-elder@riscstar.com>
2026-05-03  3:42   ` [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support Julian Braha
     [not found]   ` <CAMRc=McWXCqyv1LmWMuEMmE3HqaURx_eMD8rkDs9AJT+7W2aYw@mail.gmail.com>
2026-05-04 13:07     ` Alex Elder
     [not found] ` <20260501155421.3329862-11-elder@riscstar.com>
2026-05-05 16:38   ` [PATCH net-next 10/12] net: stmmac: " Mohd Ayaan Anwar
2026-05-05 16:46     ` Alex Elder
     [not found] ` <20260501155421.3329862-13-elder@riscstar.com>
2026-05-05 16:42   ` [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy Mohd Ayaan Anwar
2026-05-05 16:46     ` Alex Elder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox