Re: [PATCH 2/2] max9286: Add MAX9286 driver

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

 



Hi Niklas,

On 09/08/2019 13:23, Niklas Söderlund wrote:
> Hi Kieran,
> 
> On 2019-08-09 13:12:49 +0100, Kieran Bingham wrote:
>> On 09/08/2019 13:04, Niklas Söderlund wrote:
>>> Hi Kieran,
>>>
>>> Thanks for your feedback.
>>>
>>> On 2019-08-09 09:09:43 +0100, Kieran Bingham wrote:
>>>> Hi Niklas,
>>>>
>>>> This should be at least v5.
>>>
>>> I don't agree ;-) This is a "new" series where multiple streams are not 
>>> supported and there are no external dependencies. To indicate this I 
>>
>> I'm afraid there's nothing new about a version of this series with
>> support for only a single stream.
>>
>> See version 2 of my series:
>>
>> https://lore.kernel.org/linux-media/20180808165559.29957-4-kieran.bingham@xxxxxxxxxxxxxxxx/
> 
> Was not aware of this posting, then yes I do agree with you this should 
> have been v5. Sorry about that.
> 
>>
>>
>>> reset the version. I don't feel strongly about this next submission can 
>>> remedy this if you do feel strongly about it.
>>
>> I find it very confusing to have reset the version but kept exactly the
>> same patch title.
>>
>> It's a fork of the series :D
> 
> :-)
> 
> I think this proves we need to get this driver upstream so we can start 
> submitting patches towards something and not brew our our brand of stuff 
> we have laying around and find all over the place.

I agree, It's unfortunate that the direction has rotated too many times
for this driver.

Originally it was posted with VC support as that was our development,
then I removed the VC support (v2, v3) to aim to get it upstreamed.

I can't recall now why V4 occurred with VC brought back in - but perhaps
it was because the VC work was in active development at the time - and I
thought it was making progress, but it soon became a blocker - and was
not worked on until Jacopo started looking at it again recently - and
became a complete block to reposting any V5 of MAX9286 - as the VC work
was out of date and not being rebased.

Anyway, I agree - that getting a non-multi-stream capable version of
this integrated is very beneficial, so please do continue and post a v5
(or v6 if you deem more appropriate).

Please make sure all pending review comments are explored (especially
the one from Rob Herring about using an i2c-mux node) before posting
your next version though.

--
Kieran



>>>> Did you take the last v4 and work from there?
>>>> I have made changes since the last posting. Did you get an update from
>>>> my branches?
>>>>
>>>> What changes have you made to this posting compared to whichever
>>>> patch-base you have taken to start from?
>>>
>>> I took my latest known good state and diffed it with all gmsl branches i 
>>> could find picked what seamed most recent. Then I removed multiplexed 
>>> stream support, fixed a few todos in error paths to clean up notifiers 
>>> and unified naming of the private data structure.
>>
>>
>> I'll diff your version with my latest.
>>
>> There were other review comments worked on from Sakari too., and there
>> is an outstanding comment from Rob to complete on the bindings from v4.
>>
>> --
>> Kieran
>>
>>
>>
> 




[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