Re: [PATCH] spi: Add HiSilicon SPI controller driver support

[Lukas Wunner <lukas@xxxxxxxxx>]

On Mon, Mar 08, 2021 at 11:57:17AM +0800, Jay Fang wrote:
> On 2021/3/7 22:36, Lukas Wunner wrote:
> > On Mon, Mar 01, 2021 at 07:56:11PM +0800, Jay Fang wrote:
> > > +static int hisi_spi_setup(struct spi_device *spi)
> > > +{
> > > +	struct hisi_spi *hs = spi_controller_get_devdata(spi->controller);
> > > +	struct hisi_chip_data *chip;
> > > +
> > > +	/* Only alloc on first setup */
> > > +	chip = spi_get_ctldata(spi);
> > > +	if (!chip) {
> > > +		chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> > > +		if (!chip)
> > > +			return -ENOMEM;
> > > +		spi_set_ctldata(spi, chip);
> > > +	}
> > [...]
> > > +	master = devm_spi_alloc_master(dev, 0);
> > 
> > It would seem simpler to just pass "sizeof struct hisi_chip_data"
> > to devm_spi_alloc_master(), thus obviating the need to allocate
> > it on first invocation of the ->setup hook.
> 
> Thanks for your review. "struct hisi_chip_data" is the private data
> of per-spi device, not the spi controller. So I think this method
> is not suitable here. Maybe we can allocate "struct hisi_spi" in
> this way.

Ah, I was mistaken then.  Yes, allocating struct hisi_spi with
devm_spi_alloc_master() would probably make sense.

Thanks,

Lukas