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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

-- 
Sakari Ailus



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux