Re: [PATCH v4 2/5] gpio: Add support for Intel LJCA USB GPIO driver

[Ye, Xiang]

On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote:
> 
> 
> On 09.03.23 08:10, Ye Xiang wrote:
> 
> > +#define LJCA_GPIO_BUF_SIZE 60
> > +struct ljca_gpio_dev {
> > +	struct platform_device *pdev;
> > +	struct gpio_chip gc;
> > +	struct ljca_gpio_info *gpio_info;
> > +	DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM);
> > +	DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM);
> > +	DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM);
> > +	u8 *connect_mode;
> > +	/* mutex to protect irq bus */
> > +	struct mutex irq_lock;
> > +	struct work_struct work;
> > +	/* lock to protect package transfer to Hardware */
> > +	struct mutex trans_lock;
> > +
> > +	u8 obuf[LJCA_GPIO_BUF_SIZE];
> > +	u8 ibuf[LJCA_GPIO_BUF_SIZE];
> 
> And here we have a violation of DMA coherency rules.
> Basically you cannot embed buffers into other data structures
> if they can be subject to DMA.
But obuf and ibuf does not used to do DMA transfer here.
It is actually copied from or to ljca buffer to do URB transfer.
Should it still need to follow the DMA coherency rules?
> 
> 
> 
> 
> > +static int ljca_gpio_remove(struct platform_device *pdev)
> > +{
> > +	struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev);
> > +
> > +	gpiochip_remove(&ljca_gpio->gc);
> > +	ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca);
> > +	mutex_destroy(&ljca_gpio->irq_lock);
> > +	mutex_destroy(&ljca_gpio->trans_lock);
> 
> At this time, what has made sure that no work is scheduled?
Can't make sure of that. Could Adding cancel_work_sync(&ljca_gpio->work) before
mutex_destroy can address it?
> 
> > +	return 0;
> > +}
> 
> 

--
Thanks
Ye Xiang