Re: [PATCH] usb: gadget: atmel: Add SAM9X60 support

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Eugen, Zixun,

On lun., mars 24, 2025 at 11:23, Eugen Hristev <eugen.hristev@linaro.org> wrote:

> On 3/22/25 22:56, Zixun LI wrote:
>> Add compatible "microchip,sam9x60-udc" and device tree binding.
>> Compared to SAM9X5 the only difference is the DPRAM memory from the
>> USB High Speed Device Port (UDPHS) hardware block was increased,
>> so we can reuse the same endpoint data.
>> 
>> Tested on SAM9X60-Curiosity board with nfs and ums commands.
>
> Why no patch to enable it on the board as well then ?

Looking at configs/at91sam9x5ek_mmc_defconfig, I don't see
CMD_USB_MASS_STORAGE=y in there as well.

Could you elaborate on why you'd want this to be enabled as part of the
driver series?

On one hand, users has more built-in commands available and it eases the
testing. On the other hand, some users might not be interested in having
this by default. Enabling it via menuconfig is quite easy.

>
>> 
>> Signed-off-by: Zixun LI <admin@hifiphile.com>
>> ---
>>  arch/arm/dts/sam9x60.dtsi                        | 14 ++++++++++++++
>>  arch/arm/mach-at91/include/mach/atmel_usba_udc.h |  2 +-
>>  drivers/usb/gadget/atmel_usba_udc.c              |  1 +
>
> Device tree and driver changes should be separate commits.

I agree with Eugen. Could you please split this out, Zixun?

>
>
>>  3 files changed, 16 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm/dts/sam9x60.dtsi b/arch/arm/dts/sam9x60.dtsi
>> index 3b684fc63d5..96a8faf09b3 100644
>> --- a/arch/arm/dts/sam9x60.dtsi
>> +++ b/arch/arm/dts/sam9x60.dtsi
>> @@ -69,6 +69,20 @@
>>  		#size-cells = <1>;
>>  		ranges;
>>  
>> +		usb0: gadget@500000 {
>> +			compatible = "microchip,sam9x60-udc";
>> +			reg = <0x500000 0x100000>,
>> +				<0xf803c000 0x400>;
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
> Can you please reorder these to have them in the same order as in Linux
> DTS ? (easier to diff...)

Agreed with Eugen.

>
>> +			interrupts = <23 IRQ_TYPE_LEVEL_HIGH 2>;
>> +			clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_CORE 8>;
>> +			clock-names = "pclk", "hclk";
>> +			assigned-clocks = <&pmc PMC_TYPE_CORE 8>;
>> +			assigned-clock-rates = <480000000>;
>> +			status = "disabled";
>> +		};
>> +
>>  		usb1: usb@600000 {
>>  			compatible = "atmel,at91rm9200-ohci", "usb-ohci";
>>  			reg = <0x00600000 0x100000>;
>> diff --git a/arch/arm/mach-at91/include/mach/atmel_usba_udc.h b/arch/arm/mach-at91/include/mach/atmel_usba_udc.h
>> index 835b47d91ba..23c71985c90 100644
>> --- a/arch/arm/mach-at91/include/mach/atmel_usba_udc.h
>> +++ b/arch/arm/mach-at91/include/mach/atmel_usba_udc.h
>> @@ -20,7 +20,7 @@
>>  	}
>>  
>>  #if defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) || \
>> -	defined(CONFIG_AT91SAM9X5)
>> +	defined(CONFIG_AT91SAM9X5) || defined(CONFIG_SAM9X60)
>>  static struct usba_ep_data usba_udc_ep[] = {
>>  	EP("ep0", 0, 64, 1, 0, 0),
>>  	EP("ep1", 1, 1024, 2, 1, 1),
>> diff --git a/drivers/usb/gadget/atmel_usba_udc.c b/drivers/usb/gadget/atmel_usba_udc.c
>> index a77037a7094..f9326f0a7e7 100644
>> --- a/drivers/usb/gadget/atmel_usba_udc.c
>> +++ b/drivers/usb/gadget/atmel_usba_udc.c
>> @@ -1443,6 +1443,7 @@ static const struct udevice_id usba_udc_ids[] = {
>>  	{ .compatible = "atmel,at91sam9rl-udc" },
>>  	{ .compatible = "atmel,at91sam9g45-udc" },
>>  	{ .compatible = "atmel,sama5d3-udc" },
>> +	{ .compatible = "microchip,sam9x60-udc" },
>>  	{}
>>  };
>>