Re: [PATCH v3 03/25] mtd: spi-nor: Introduce spi_nor_set_mtd_info()

[Pratyush Yadav <p.yadav@ti.com>]

On 17/11/21 02:36PM, Tudor.Ambarus@microchip.com wrote:
> On 11/16/21 8:11 PM, Pratyush Yadav wrote:
> 
> >>
> >>> - spi_nor_try_unlock_all(), which is called by spi_nor_init(). I don't
> >>>   think it actually uses any values you initialize here but still worth
> >>>   pointing out.
> >>
> >> we are safe here, the pointer to mtd is used just to get the pointer to
> >> nor.
> > 
> > Yeah, but who knows if that might change some time later. I would prefer
> > we don't use a member we haven't initialized yet.
> 
> If it weren't for the SPI NOR controller drivers that use
> spi_nor_scan(), I would put the spi_nor_set_mtd_info() just
> above the mtd_device_register(). It will indicate that no mtd_info
> field is used up to that point, less things to worry about.
> spi_nor_try_unlock_all() calls
> 	spi_nor_unlock(&nor->mtd, 0, nor->params->size);
> I can't see for now if we will ever need some specific mtd_info
> parameter. I would say that we won't, we're just unlocking the full
> flash, every info we would need we can obtain from NOR. The discussion
> would be different if it were about mtd partitions, but it isn't, we're
> dealing with the entire flash.
> 
> Would you accept the place where I put spi_nor_set_mtd_info() if I add
> a comment before calling it? Something like:
> /* No mtd_info fields are used up to this point. */
> spi_nor_set_mtd_info();

I see that everything that spi_nor_set_mtd_info() needs is set by the 
time spi_nor_init_params() is finished. Everything after that is 
concerned about selecting the protocol and sending the init commands to 
the flash. So why can't you call it right after spi_nor_init_params()? 
That and updating spi_nor_spimem_check_op() and spi_nor_set_addr_width() 
to use nor->params->size instead of nor->mtd.size should do the trick.

I think that it is implied that mtd_info fields are not being used until 
they are initialized so I don't think the comment itself is of much use, 
but I don't care much about it either way.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel