On Tue, Nov 23, 2021 at 12:09:54AM +0200, Andy Shevchenko wrote: > On Mon, Nov 22, 2021 at 4:34 AM LH.Kuo <lhjeff911@xxxxxxxxx> wrote: > > +// slave only. usually called on driver remove > Why is it so? > Also find use of proper English grammar (capitalization, periods, etc. > Ditto for all your comments. Please don't go overboard on changes you're requesting, the important thing with comments is that they're intelligible. People have different levels of skill with English and that's totally fine, it's much better that people feel able to write comments than that they stop doing so because they are concerned about issues with their foreign language skills. > > + unsigned long flags; > > + struct sp7021_spi_ctlr *pspim = dev; > > + u32 fd_status = 0; > > + unsigned int tx_len, rx_cnt, tx_cnt, total_len; > > + bool isrdone = false; > Reversed xmas tree order here and everywhere else. Again, please don't go overboard - this isn't a general requirement for kernel code, some parts of the kernel do want it but outside of those areas it's not something that we should be asking for (and TBH even when it is required you should explain what it is, it's not as easy to search for as it could be). I certainly don't care. > > + if (of_property_read_bool(pdev->dev.of_node, "spi-slave")) > > + mode = SP7021_SLAVE_MODE; > There is no need to check of_node for this call. OTOH if we are checking it anyway it doesn't hurt to have all the property reads in the check for of_node. Either way it doesn't fundamentally matter.
Attachment:
signature.asc
Description: PGP signature
