Re: [PATCH 2/2] clocksource: tegra: Register watchdog device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/14/2014 04:42 AM, Thierry Reding wrote:
On Mon, Oct 13, 2014 at 09:42:10AM -0700, Guenter Roeck wrote:
On 10/13/2014 05:05 AM, Thierry Reding wrote:
From: Thierry Reding <treding@xxxxxxxxxx>

The watchdog timer is part of the timer controller block on Tegra. In
order to avoid access to the same registers from two drivers, register
the watchdog device from the clocksource driver.

Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>

Does that really make sense ?

A couple of callbacks into the clock driver to implement register accesses
might be a better approach.

I guess that would be a valid approach as well. It has the downside of
requiring the addition of at least two globally visible symbols to the
kernel. It also means that we'd need to somehow pass around a struct
device for diagnostic messages and so on. Dealing with all of that seems
like much more of a burden than this.

Also if you look at the diffstat this approach allows us to get rid of
80 lines of code. Adding a custom mechanism to share the register space
would be more likely to result in a positive diffstat.

FWIW, (although I haven't read the patches), the general idea of registering a single driver for each HW block makes sense to me. While we've split up HW blocks into separate drivers in the past, I think that's just made things more complex without much benefit, so I think those decisions were a mistake in retrospect. If we do actually need to split things up into separate drivers, we should use MFD rather than multiple unrelated top-level drivers. That way, we will have a single top-level driver that gets instantiated from a single DT node (or platform device in a board file or ACPI thing or ...)
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux