ION DTS changes for HiKey in -next

ION DTS changes for HiKey in -next

[Mark Brown]

Hi Greg,

I was just looking at DTs in -next and noticed that there is a patch
59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
which adds at DT doing something for ION.  Are we sure this should be
going into the main production DT?  The bindings haven't been reviewed
as far as I can tell, the matching driver is only in staging and hasn't
been posted upstream.

Thanks,
Mark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160107/e859508e/attachment.sig>

ION DTS changes for HiKey in -next

[Greg Kroah-Hartman]

On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
> Hi Greg,
> 
> I was just looking at DTs in -next and noticed that there is a patch
> 59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
> which adds at DT doing something for ION.  Are we sure this should be
> going into the main production DT?  The bindings haven't been reviewed
> as far as I can tell, the matching driver is only in staging and hasn't
> been posted upstream.

Isn't "staging" upstream enough for this?  :)

If not, let me know and I'll be glad to drop this patch, it was on the
list for a while with no objections.

thanks,

greg k-h

ION DTS changes for HiKey in -next

[Mark Brown]

On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
> On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:

> > I was just looking at DTs in -next and noticed that there is a patch
> > 59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
> > which adds at DT doing something for ION.  Are we sure this should be
> > going into the main production DT?  The bindings haven't been reviewed
> > as far as I can tell, the matching driver is only in staging and hasn't
> > been posted upstream.

> Isn't "staging" upstream enough for this?  :)

I wouldn't have thought so, DTs are supposed to be an ABI so we want
proper review and having had a quick glance this doesn't look like it's
a hardware description so it's not clear to me it should be in DT at all.

> If not, let me know and I'll be glad to drop this patch, it was on the
> list for a while with no objections.

Which list?  I did look for the original thread to reply to but it
seemed to be only on the Linux driver project list.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160108/f3396d79/attachment.sig>

ION DTS changes for HiKey in -next

[Mark Rutland]

On Fri, Jan 08, 2016 at 12:44:39PM +0000, Mark Brown wrote:
> On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
> > On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
> 
> > > I was just looking at DTs in -next and noticed that there is a patch
> > > 59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
> > > which adds at DT doing something for ION.  Are we sure this should be
> > > going into the main production DT?  The bindings haven't been reviewed
> > > as far as I can tell, the matching driver is only in staging and hasn't
> > > been posted upstream.
> 
> > Isn't "staging" upstream enough for this?  :)
> 
> I wouldn't have thought so, DTs are supposed to be an ABI so we want
> proper review and having had a quick glance this doesn't look like it's
> a hardware description so it's not clear to me it should be in DT at all.

Indeed.

The driver and the binding before that don't really belong either,
I would have NAK'd those on devicetree at vger.kernel.org, though it
appears I either missed them or they never made it to that list.

>From my PoV there should not be a platform-specific ION binding. If we
need one at all, people should work on the proposed generic binding [1]
or figure out how to do this with the existing reserved-memory bindings.

Thanks,
Mark.

[1] https://lkml.org/lkml/2015/10/6/854

ION DTS changes for HiKey in -next

[Laura Abbott]

On 1/8/16 5:55 AM, Mark Rutland wrote:
> On Fri, Jan 08, 2016 at 12:44:39PM +0000, Mark Brown wrote:
>> On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
>>> On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
>>
>>>> I was just looking at DTs in -next and noticed that there is a patch
>>>> 59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
>>>> which adds at DT doing something for ION.  Are we sure this should be
>>>> going into the main production DT?  The bindings haven't been reviewed
>>>> as far as I can tell, the matching driver is only in staging and hasn't
>>>> been posted upstream.
>>
>>> Isn't "staging" upstream enough for this?  :)
>>
>> I wouldn't have thought so, DTs are supposed to be an ABI so we want
>> proper review and having had a quick glance this doesn't look like it's
>> a hardware description so it's not clear to me it should be in DT at all.
>
> Indeed.
>
> The driver and the binding before that don't really belong either,
> I would have NAK'd those on devicetree at vger.kernel.org, though it
> appears I either missed them or they never made it to that list.
>
>  From my PoV there should not be a platform-specific ION binding. If we
> need one at all, people should work on the proposed generic binding [1]
> or figure out how to do this with the existing reserved-memory bindings.
>
> Thanks,
> Mark.
>
> [1] https://lkml.org/lkml/2015/10/6/854

I posted v2 back in November
(http://article.gmane.org/gmane.linux.drivers.driver-project.devel/80475)
but there wasn't much in the way of review comments. More feedback there
would be appreciated or I can resend.

Thanks,
Laura

ION DTS changes for HiKey in -next

[Greg Kroah-Hartman]

On Fri, Jan 08, 2016 at 10:14:35AM -0800, Laura Abbott wrote:
> On 1/8/16 5:55 AM, Mark Rutland wrote:
> >On Fri, Jan 08, 2016 at 12:44:39PM +0000, Mark Brown wrote:
> >>On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
> >>>On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
> >>
> >>>>I was just looking at DTs in -next and noticed that there is a patch
> >>>>59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
> >>>>which adds at DT doing something for ION.  Are we sure this should be
> >>>>going into the main production DT?  The bindings haven't been reviewed
> >>>>as far as I can tell, the matching driver is only in staging and hasn't
> >>>>been posted upstream.
> >>
> >>>Isn't "staging" upstream enough for this?  :)
> >>
> >>I wouldn't have thought so, DTs are supposed to be an ABI so we want
> >>proper review and having had a quick glance this doesn't look like it's
> >>a hardware description so it's not clear to me it should be in DT at all.
> >
> >Indeed.
> >
> >The driver and the binding before that don't really belong either,
> >I would have NAK'd those on devicetree at vger.kernel.org, though it
> >appears I either missed them or they never made it to that list.
> >
> > From my PoV there should not be a platform-specific ION binding. If we
> >need one at all, people should work on the proposed generic binding [1]
> >or figure out how to do this with the existing reserved-memory bindings.
> >
> >Thanks,
> >Mark.
> >
> >[1] https://lkml.org/lkml/2015/10/6/854
> 
> I posted v2 back in November
> (http://article.gmane.org/gmane.linux.drivers.driver-project.devel/80475)
> but there wasn't much in the way of review comments. More feedback there
> would be appreciated or I can resend.

I still have those in my queue, but was waiting for someone to chime in
on them.  I guess if no one objected, that means I should accept them?
:)

thanks,

greg k-h

ION DTS changes for HiKey in -next

[Laura Abbott]

On 1/8/16 9:05 PM, Greg Kroah-Hartman wrote:
> On Fri, Jan 08, 2016 at 10:14:35AM -0800, Laura Abbott wrote:
>> On 1/8/16 5:55 AM, Mark Rutland wrote:
>>> On Fri, Jan 08, 2016 at 12:44:39PM +0000, Mark Brown wrote:
>>>> On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
>>>>> On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
>>>>
>>>>>> I was just looking at DTs in -next and noticed that there is a patch
>>>>>> 59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
>>>>>> which adds at DT doing something for ION.  Are we sure this should be
>>>>>> going into the main production DT?  The bindings haven't been reviewed
>>>>>> as far as I can tell, the matching driver is only in staging and hasn't
>>>>>> been posted upstream.
>>>>
>>>>> Isn't "staging" upstream enough for this?  :)
>>>>
>>>> I wouldn't have thought so, DTs are supposed to be an ABI so we want
>>>> proper review and having had a quick glance this doesn't look like it's
>>>> a hardware description so it's not clear to me it should be in DT at all.
>>>
>>> Indeed.
>>>
>>> The driver and the binding before that don't really belong either,
>>> I would have NAK'd those on devicetree at vger.kernel.org, though it
>>> appears I either missed them or they never made it to that list.
>>>
>>>  From my PoV there should not be a platform-specific ION binding. If we
>>> need one at all, people should work on the proposed generic binding [1]
>>> or figure out how to do this with the existing reserved-memory bindings.
>>>
>>> Thanks,
>>> Mark.
>>>
>>> [1] https://lkml.org/lkml/2015/10/6/854
>>
>> I posted v2 back in November
>> (http://article.gmane.org/gmane.linux.drivers.driver-project.devel/80475)
>> but there wasn't much in the way of review comments. More feedback there
>> would be appreciated or I can resend.
>
> I still have those in my queue, but was waiting for someone to chime in
> on them.  I guess if no one objected, that means I should accept them?
> :)

Before accepting them. I'd rather have some acknowledgement from the DT
maintainers that these bindings are moving in the right direction or
at least a better idea than the per platform bindings.

>
> thanks,
>
> greg k-h
>

Thanks,
Laura

ION DTS changes for HiKey in -next

[Mark Rutland]

On Fri, Jan 08, 2016 at 10:14:35AM -0800, Laura Abbott wrote:
> On 1/8/16 5:55 AM, Mark Rutland wrote:
> >On Fri, Jan 08, 2016 at 12:44:39PM +0000, Mark Brown wrote:
> >>On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
> >>>On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
> >>
> >>>>I was just looking at DTs in -next and noticed that there is a patch
> >>>>59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
> >>>>which adds at DT doing something for ION.  Are we sure this should be
> >>>>going into the main production DT?  The bindings haven't been reviewed
> >>>>as far as I can tell, the matching driver is only in staging and hasn't
> >>>>been posted upstream.
> >>
> >>>Isn't "staging" upstream enough for this?  :)
> >>
> >>I wouldn't have thought so, DTs are supposed to be an ABI so we want
> >>proper review and having had a quick glance this doesn't look like it's
> >>a hardware description so it's not clear to me it should be in DT at all.
> >
> >Indeed.
> >
> >The driver and the binding before that don't really belong either,
> >I would have NAK'd those on devicetree at vger.kernel.org, though it
> >appears I either missed them or they never made it to that list.
> >
> > From my PoV there should not be a platform-specific ION binding. If we
> >need one at all, people should work on the proposed generic binding [1]
> >or figure out how to do this with the existing reserved-memory bindings.
> >
> >Thanks,
> >Mark.
> >
> >[1] https://lkml.org/lkml/2015/10/6/854
> 
> I posted v2 back in November
> (http://article.gmane.org/gmane.linux.drivers.driver-project.devel/80475)
> but there wasn't much in the way of review comments. More feedback there
> would be appreciated or I can resend.

A resend would be appeciated, as that's easier to reply to.

I'm still a little fuzzy on why this can't be done with reserved-memory,
but that's a discussion better left for the thread with the patches.

Thanks,
Mark.

ION DTS changes for HiKey in -next

[Greg Kroah-Hartman]

On Fri, Jan 08, 2016 at 01:55:52PM +0000, Mark Rutland wrote:
> On Fri, Jan 08, 2016 at 12:44:39PM +0000, Mark Brown wrote:
> > On Thu, Jan 07, 2016 at 09:02:14PM -0800, Greg Kroah-Hartman wrote:
> > > On Thu, Jan 07, 2016 at 05:37:44PM +0000, Mark Brown wrote:
> > 
> > > > I was just looking at DTs in -next and noticed that there is a patch
> > > > 59dfafd03fc (arm64: dts: Add dts files to enable ION on Hi6220 SoC)
> > > > which adds at DT doing something for ION.  Are we sure this should be
> > > > going into the main production DT?  The bindings haven't been reviewed
> > > > as far as I can tell, the matching driver is only in staging and hasn't
> > > > been posted upstream.
> > 
> > > Isn't "staging" upstream enough for this?  :)
> > 
> > I wouldn't have thought so, DTs are supposed to be an ABI so we want
> > proper review and having had a quick glance this doesn't look like it's
> > a hardware description so it's not clear to me it should be in DT at all.
> 
> Indeed.
> 
> The driver and the binding before that don't really belong either,
> I would have NAK'd those on devicetree at vger.kernel.org, though it
> appears I either missed them or they never made it to that list.
> 
> From my PoV there should not be a platform-specific ION binding. If we
> need one at all, people should work on the proposed generic binding [1]
> or figure out how to do this with the existing reserved-memory bindings.

Ok, I've now reverted that commit, thanks for letting me know.

greg k-h