Re: [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 3767 bytes --]

On 24/11/14 19:39, Mark Brown wrote:

> OK...  this is all telling me that I *really* need to scrub this in
> detail.  It's all sounding very vague, it's an area which seems to cause
> lots of problems and I don't want to be sitting here next time around
> trying to figure out if another rewrite makes things better or worse or
> if another driver should look similar or different.

I was vague as I don't have the details. I don't know much about the
audio side, except from the users side. Jyri can perhaps fill in the
details.

But a proper review for sound/ side would be appreciated.

For what it's worth, I can say from a user's perspective that with this
series the OMAP HDMI audio finally seems to work so that I'm satisfied
with it. While there's still things to improve, it works and I can keep
it enabled in the kernel while developing the video side and it doesn't
get on my way.

>> The whole series is about HDMI audio, not video.
> 
> You know exactly what I mean - the early patches are in drivers/video,
> don't touch anything outside of that and have no obvious dependencies.

My point was, there's no particular reason to apply those patches
individually. It's an independent series, about HDMI audio, and applying
only some of the earlier patches does not improve the kernel in visible
manner.

>> And in any case, I don't like applying individual patches from a series.
>> Usually that just complicates things. If I would apply some of the early
>> patches to fbdev, then this series would no longer work on plain
>> mainline kernel, and would instead depend on fbdev tree. The situation
> 
> But I thought everyone was saying this hardware doesn't work anyway and
> that you want to apply all these changes to fbdev?

I don't particularly _want_ to apply these via fbdev, but I think it's
the easiest way. Most of the files touched are in drivers/video/, so
hopefully merging via fbdev reduces the chances of conflicts.

And this needs your ack before it can be merged via fbdev tree. Until
you've said that you're fine getting this via fbdev, we don't know how
this will be merged, and I can't create dependencies to fbdev.

> This means we're all then stuck reading reposts of the same enormous
> series over and over again - as a reviewer a really big series that
> appears from the subject lines to be mostly about another system is
> really offputting.  If you're going to do something like this please at

Well, yes, I see your point. And I agree that patches that the rest of
the series does not depend on should normally be applied individually if
the series starts getting multiple revisions (although even those
patches could cause conflicts if the rest of the patches touch code nearby).

But that's not the case here, as all the patches are needed to get a
working HDMI audio.

> least reply to the messages, that way it's clearer that there's not
> going to be a dependency problem getting the patches applied (which is
> part of what makes things offputting).

Yes, lack of communication was my mistake.

In any case, how do you want to proceed? As I said, I very much would
like to get this into the next merge window and from my point of view
the current versions looks good. If you find something to be fixed in
the sound/ side, and you're fine with merging this via fbdev, I can
start merging the earlier patches in the series so that the next
revision is easier to review.

But the merge window is getting close. If you find something very wrong
with the series, we can skip this merge window. But if there are only
issues that can be easily fixed with follow-up patches, I'd rather merge
this revision than do more full review rounds.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]