Hi Daniel,
On Mon, Nov 05, 2018 at 09:19:35PM +0100, Daniel Golle wrote:
> Hi Paul,
>
> thank you for the review!
You're very welcome :)
> On Mon, Nov 05, 2018 at 06:36:16PM +0000, Paul Burton wrote:
> > Is there an in-tree user for these?
>
> Not yet, OpenWrt's out-of-tree Ethernet driver needs the already
> existing int mt7620_get_eco(void) and is going to be upstreamed once
> MT7530 DSA for has been completed to work with MT7621. See the driver
> in [1].
>
> The two newly introduced accessors are going to be used by the in-tree
> rt2x00 driver which gained support for the RT6352 wireless core
> included in that SoC recently. In order to be able to carry out tuning
> in the same way the vendor driver does, rt2x00 will need to access the
> pkg and chipver fields. See [2] for example.
>
> >
> > Looking at it I don't see any in-tree code which uses the existing
> > mt7620_get_eco() function. I'm not fond of adding code which isn't used
> > at all in-tree, I'd much rather we either:
> >
> > 1) Get the driver that needs these upstreamed, and these functions
> > could be added at the same time.
> >
> > or
> >
> > 2) Keep functions only used by out-of-tree code out-of-tree.
>
> I understand your concerns with regard to the mt7620_get_eco(void)
> function which is currently only used by the out-of-tree Ethernet
> driver. However, the to-be-introduced functions mt7620_get_pkg(void)
> and mt7620_get_chipver(void) are to be used in-tree by
> drivers/net/wireless/ralink/rt2x00 in the very near future. I just
> wanted to consult whether the introductions of such accessors is
> generally acceptable before implementing the changes in rt2x00.
OK, then my suggestion is that you include this patch in the rt2x00
series you create & feel free to add:
Acked-by: Paul Burton <paul.burton@xxxxxxxx>
Presuming that the functions are used by something else in the patch
series.
Does that sound OK to you?
Thanks,
Paul
