On Mon, Aug 17, 2020 at 02:55:38PM +0300, Sakari Ailus wrote: > On Mon, Aug 17, 2020 at 02:33:22PM +0300, Andy Shevchenko wrote: > > On Mon, Aug 17, 2020 at 02:27:47PM +0300, Sakari Ailus wrote: > > > On Mon, Aug 17, 2020 at 02:20:06PM +0300, Andy Shevchenko wrote: > > > > On Mon, Aug 17, 2020 at 02:13:16PM +0300, Sakari Ailus wrote: > > > > > On Mon, Aug 17, 2020 at 12:44:36PM +0300, Andy Shevchenko wrote: > > > > > > On Mon, Aug 17, 2020 at 05:27:33PM +0800, Bingbu Cao wrote: > > > > > > > On 8/15/20 12:30 AM, Andy Shevchenko wrote: > > > > > > > > We may use special helper macro to poll IO till condition or timeout occurs. > > > > > > > > > > > > > > + ret = readl_poll_timeout(dma, value, value & CIO2_CDMAC0_DMA_HALTED, 4000, 2000000); > > > > > > > > > > > > > > This line is too long, need a break, others look good for me. > > > > > > > > > > > > checkpatch doesn't complain, but if you insist, I'll split it in v2. > > > > > > > > > > The coding style hasn't changed, it's just the checkpatch.pl warning that > > > > > has. > > > > > > > > Joe, it seems we have inconsistency now between checkpatch and coding style. > > > > Shouldn't we revert 100 limit warning to 80? > > > > > > There are sometimes genuine reasons for having longer lines than 80, and > > > depending on the code, that happens more often in some places than > > > elsewhere. This tended to generate lots of checkpatch.pl warnings in the > > > past. > > > > > > While I didn't see the patch removing the 80 chars per line limit until it > > > made the news, I think it was a quite reasonable compromise. > > > > But doesn't it make harder life for reviewers like you? You have to keep in > > mind all these inconsistencies and rule either way. > > > > That said, we either would fix the doc, or revert the checkpatch change. > > I don't think the documentation should be changed. There *are* reasons for > the 80-column limit. Although the exact number can be connected to the > width of traditional text terminals, using very long lines makes reading > text harder. IOW, there's a connection to the same reason why wide > newspaper articles are split into several columns. > Would it be reasonable if checkpatch.pl warned about exceeding 80-column > limits if such lines are rare or nonexistent in the same file? I don't think so. If somebody decides to *gradually* move to 100 limit in the driver, it will be painful with all these new old warnings. -- With Best Regards, Andy Shevchenko
