mbox series

[v2,0/1] Input: add gamecube adapter support

Message ID PA4P189MB146928AA17984371B61EB909FAE62@PA4P189MB1469.EURP189.PROD.OUTLOOK.COM
Headers show
Series Input: add gamecube adapter support | expand

Message

Milas Robin May 9, 2024, 1:12 a.m. UTC 
> > +struct ngc_data {
> > +	char phys[64];
> > +
> > +	struct usb_device *udev;
> > +	struct usb_interface *intf;
> > +
> > +	struct urb *irq_in;
> > +	u8 *idata;
> > +	dma_addr_t idata_dma;
> > +	spinlock_t idata_lock;
> > +
> > +	struct urb *irq_out;
> > +	struct usb_anchor irq_out_anchor;
> > +	u8 *odata;
> > +	dma_addr_t odata_dma;
> > +	spinlock_t odata_lock;		/* output data */
> > +	bool irq_out_active;		/* we must not use an active URB */
> 
> I think all of this starting with irq_out should be under #ifdef
> CONFIG_JOYSTICK_NGC_FF.

Unfortunately, a first packet must be sent to the adapter before it start
sending controllers values.

I was able to remove the irq_out_active field with some rewrite.
Technically it would be possible to remove the odata_lock field too but with
the current code I have it would mean putting a lockdep_assert_held into
an ifndef which feels weird to me.

> > +static int ngc_queue_rumble(struct ngc_data *gdata)
> > +{
> > +	int error;
> > +
> 
> Please check irq_out_active flag here and also provide a stub for
> !CONFIG_JOYSTICK_NGC_FF so you do not need to sprinkle #ifdef checks..

I changed the way my code is cut to reduce the number of #ifdef as best as I can

I incorporated every changes on this new version

Thank you for your review

Robin