From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dr. Philipp Tomsich Date: Thu, 26 Apr 2018 09:37:22 +0200 Subject: [U-Boot] [U-Boot,2/3] rockchip: rk3188: add timer3 node In-Reply-To: References: <1524021226-18474-2-git-send-email-kever.yang@rock-chips.com> <64e21a24-971c-8cba-6669-0b5f8bac49dc@rock-chips.com> Message-ID: <13201095-84D0-4056-9C85-48C3AA9DC6D9@theobroma-systems.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de > On 26 Apr 2018, at 09:14, Dr. Philipp Tomsich wrote: > > >> On 26 Apr 2018, at 04:50, Kever Yang wrote: >> >> Hi Philipp, >> >> On 04/25/2018 06:17 PM, Dr. Philipp Tomsich wrote: >>> Kever, >>> >>>> On 25 Apr 2018, at 12:04, Philipp Tomsich wrote: >>>> >>>>> Add dts node for timer3. >>>>> Because of the rockchip timer can only KNOWN "dtd_rockchip_rk3368_timer" >>>>> with OF_PLATDATA enable, so we override its compatible to >>>>> "rockchip,rk3368-timer". >>>>> >>>>> Signed-off-by: Kever Yang >>>>> --- >>>>> >>>>> arch/arm/dts/rk3188-radxarock-u-boot.dtsi | 6 ++++++ >>>>> arch/arm/dts/rk3188.dtsi | 6 ++++++ >>>>> 2 files changed, 12 insertions(+) >>>>> >>>> Acked-by: Philipp Tomsich >>> This looks like a work-around that we’ll have to revert eventually. >>> I would instead extend the driver to recognise the ‘rockchip,rk3188-timer’ >>> and ‘rockchip,rk3288-timer’ as well. >>> >>> Please confirm that both these .compatible strings should also be handled >>> by the same driver and I’ll make the change when applying this series. >> >> Do you mean you want patch like this? I do not like add lots >> #ifdef/#elif like this. >> +++ b/drivers/timer/rockchip_timer.c >> @@ -17,7 +17,11 @@ DECLARE_GLOBAL_DATA_PTR; >> >> #if CONFIG_IS_ENABLED(OF_PLATDATA) >> struct rockchip_timer_plat { >> +#ifdef CONFIG_ROCKCHIP_RK3368 >> struct dtd_rockchip_rk3368_timer dtd; >> +#elif CONFIG_ROCKCHIP_RK3188 >> + struct dtd_rockchip_rk3188_timer dtd; >> +#endif >> }; >> #endif > > The OF_PLATDATA implementation is really broken in this regard: the > only fix I can think of would be to emit "struct dtd_…” for every compatible > listed in the DTS. I stand corrected: the OF_PLATDATA implementation (i.e. dtoc) already created #define entries for all aliases in the compatible-list of a DTS node. We’ll have to clean up our DTS files and this driver in the next release cycle and simply need to make sure that a common compatible is found across all devices (and the driver expects that common structure name). —Philipp.