Messages in this thread | | | From | Saravana Kannan <> | Date | Tue, 28 Apr 2020 11:23:12 -0700 | Subject | Re: [PATCH 1/2] drivers/clocksource/timer-of: Remove __init markings |
| |
On Tue, Apr 28, 2020 at 12:02 AM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > > Hi Saravana,
You were replying to Sandeep :)
> On 28/04/2020 00:17, Sandeep Patil wrote: > > Hi Daniel, > > > > On Mon, Apr 27, 2020 at 10:50:24PM +0200, Daniel Lezcano wrote: > >> On 27/04/2020 22:12, Saravana Kannan wrote: > >>> On Mon, Apr 27, 2020 at 1:09 PM Daniel Lezcano > >>> <daniel.lezcano@linaro.org> wrote: > >>>> > >>>> On 27/04/2020 21:04, Saravana Kannan wrote: > >>>>> On Mon, Apr 27, 2020 at 10:13 AM Daniel Lezcano > >>>>> <daniel.lezcano@linaro.org> wrote: > >>>>>> > >>>>>> On 13/04/2020 04:55, Baolin Wang wrote: > >>>>>>> Hi Daniel, > >>>>>>> > >>>>>>> On Tue, Mar 24, 2020 at 1:59 PM Baolin Wang <baolin.wang7@gmail.com> wrote: > >>>>>>>> > >>>>>>>> From: Saravana Kannan <saravanak@google.com> > >>>>>>>> > >>>>>>>> This allows timer drivers to be compiled as modules. > >>>>>>>> > >>>>>>>> Signed-off-by: Saravana Kannan <saravanak@google.com> > >>>>>>>> Signed-off-by: Baolin Wang <baolin.wang7@gmail.com> > >>>>>>> > >>>>>>> Do you have any comments for this patch set? Thanks. > >>>>>> > >>>>>> If my understanding is correct, this patch is part of the GKI picture > >>>>>> where hardware drivers are converted to modules. > >>>>>> > >>>>>> But do we really want to convert timer drivers to modules ? > >>>>>> > >>>>>> Is the core time framework able to support that (eg. load + unload ) > >>>>> > >>>>> So this will mainly be used for secondary timers that the system > >>>>> supports. Not for the main one that's set up during early boot for > >>>>> sched timer to work. For the primary timer during boot up, we still > >>>>> expect that to be the default ARM timer and don't want/expect that to > >>>>> be a module (it can't be). > >>>> > >>>> My question is about clockevents_config_and_register() for instance, is > >>>> there a function to unregister in the core framework ? > >>> > >>> We can just have these modules be "permanent" modules that can't be > >>> unloaded. They just need to not implement module_exit(). > >> > >> You are right. > >> > >> I can understand the goal of making everything as much modular as possible. > >> > >> But TBH, I have a bad feeling about this. Sounds like GKI will give the > >> opportunity to companies to stop upstreaming their drivers and favoring > >> fragmentation like what we had several years ago. Not sure it is a good > >> thing, especially for upstream SoC support. > > > > ... and that is a very valid concern too IMO. > > > > However, the way we see it, as things stand today, we don't even know what > > goes into Linux on all android phones out there. We know what we add, as part > > of the AOSP kernel, however, what actually runs on the device is normally > > about a million lines of code changes on top of what we do. > > > > So, for the GKI parts, we are doing the following > > > > 1. Making the peripheral drivers modules also means the GKI must have all the > > core framework changes built-in. This gets us the list of core kernel changes > > that ship on Android devices so we can work on upstreaming them OR find the > > appropriate alternative. For Android, that answers the canonical > > - "Where is the use case?" question coming from anyone. > > > > You can see the list of these out-of-tree changes is growing by the day in > > AOSP right now[1]. Its there for everyone to find in exactly *one place*. > > Note that, almost all of those patches have been posted on the list already. > > That's the first pre-requisite for any change that goes into AOSP kernel[2]. > > > > 2. Once we have a core kernel that *truly* works on all Android devices, we > > will have built up list of changes we want to upstream (or anyone can pick > > them from our public tree). Android will still continue to move to newer > > kernel versions easily (may be at a difference cadence ..) > > > > 3. About the incentive for upstream SoC support: As part of GKI, we are not > > promising a forever stable kernel<->module interface. We still change it each > > year. The *best way* for anyone to have their SoC / peripheral supported is > > _still_ "going upstream". In fact, we advertise it as such[2]. The modularity > > aspect just brings a much needed flexibility for execution. The flexibility > > is needed given the number of stakeholders involved just in the kernel as of > > today. (Its a mix of Upstream, Google, SoC manufacturer, device maker and > > many other small parts). > > > > > > 4. We also haven't been so keen on the "unloading" of a module. We know there > > were subsystems where unloading may not work as expected. Then again, to my > > knowledge, nobody has been stress testing with 500+ different modules that > > register to all core frameworks being loaded and unloaded at random times. > > Even if someone did, we just didn't think its worth the hassle or time at > > this moment. Unloading the module didn't buy us anything. (Although, I do get > > the point about "correctness" -- so it shouldn't also be obviously broken) > > That was my understanding of the GKI, thanks for confirming. > > Putting apart the non-technical aspect of these changes, the benefit I > see is the memory usage optimization regarding the single kernel image. > > With the ARM64 defconfig, multiple platforms and their corresponding > drivers are compiled-in. It results in a big kernel image which fails to > load because of overlapping on DT load address (or something else). When > that is detected, it is fine to adjust the load addresses, otherwise it > is painful to narrow down the root cause. > > In order to prevent this, we have to customize the defconfig each > version release.
Sorry, I'm not sure I understand where you are going with this. Are you agreeing to pick up this change?
-Saravana
| |