Re: [PATCH v1 6/7] media: ipu3-cio2: Use readl_poll_timeout() helper

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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