Messages in this thread | ![/](/images/icornerl.gif) | | Date | Tue, 14 Mar 2023 09:42:49 -0700 | Subject | Re: [PATCH v2] watchdog: ebc-c384_wdt: Migrate to the regmap API | From | Guenter Roeck <> |
| |
On 3/14/23 09:31, William Breathitt Gray wrote: > On Tue, Mar 14, 2023 at 05:50:42PM +0200, Andy Shevchenko wrote: >> On Tue, Mar 14, 2023 at 11:29:16AM -0400, William Breathitt Gray wrote: >>> The regmap API supports IO port accessors so we can take advantage of >>> regmap abstractions rather than handling access to the device registers >>> directly in the driver. >> >> ... >> >>> - Utilize watchdog_set_drvdata() and watchdog_get_drvdata() >> >> I'm wondering why you can't use dev_get_regmap() instead. > > We can set `wdd->parent = dev` in ebc_c384_wdt_probe(), and then use > `dev_get_regmap(wdev->parent)` to retrieve the regmap. The only downside > I see if perhaps the added latency a call to devres_find(), whereas > using watchdog_get_drvdata() is just a pointer dereference. > > I'm indifferent to either choice, so if Guenter or Wim have a preference > here I'll follow their decision. >
I am not inclined to accept a patch which calls dev_get_regmap() more than once. It is not just added latency, it is unnecessarily executing a lot of code. Maybe that call is abused nowadays, and/or maybe people do not care about wasting CPU cycles anymore, but that is not its intended use case.
>> >>> - Map watchdog control registers based on offset 0x1 and adjust regmap >>> configurations accordingly; offset 0x0 is unused in this driver so we >>> should avoid unnecessary exposure of it >> >> I'm wondering what bad could happen if you expose it. > > The WINSYSTEMS EBC-C384 documentation I have does not specify what > offset 0x0 does (nor offsets 0x3-0x4), so I don't know if there are side > effects to reading those addresses. Really, I'm just avoiding the hassle > of writing an explicit precious registers table for those offsets by not > exposing them at all. >
Counter questions: - What would be the purpose of exposing register addresses if they are not needed ? - What bad can happen by _not_ exposing those register addresses ?
Guenter
| ![\](/images/icornerr.gif) |