Messages in this thread | | | Subject | Re: [PATCH v4] reset: add support for non-DT systems | From | David Lechner <> | Date | Tue, 20 Feb 2018 10:40:13 -0600 |
| |
On 02/20/2018 04:39 AM, Philipp Zabel wrote: > Hi Bartosz, David, > > On Mon, 2018-02-19 at 18:21 -0600, David Lechner wrote: >> On 02/19/2018 10:58 AM, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> >>> >>> The reset framework only supports device-tree. There are some platforms >>> however, which need to use it even in legacy, board-file based mode. >>> >>> An example of such architecture is the DaVinci family of SoCs which >>> supports both device tree and legacy boot modes and we don't want to >>> introduce any regressions. >>> >>> We're currently working on converting the platform from its hand-crafted >>> clock API to using the common clock framework. Part of the overhaul will >>> be representing the chip's power sleep controller's reset lines using >>> the reset framework. >>> >>> This changeset extends the core reset code with new reset lookup >>> structures. Each lookup table contains a set of lookup entries which >>> allow the reset core to associate reset lines with devices (by >>> comparing the dev_id and con_id strings). >>> >>> Machine code can register a set of reset lines using this lookup table >>> and concerned devices can access them using the regular reset_control >>> API. >>> >>> The new lookup function is only called as a fallback in case the >>> of_node field is NULL and doesn't change anything for current users. >>> >>> Tested with a dummy reset driver with several lookup entries. >>> >>> An example lookup table can be found below: >>> >>> static struct platform_device foobar_reset_dev = { >>> .name = "foobar-reset", >>> }; >>> >>> static struct reset_lookup_entry foobar_reset_lookup_entries[] = { >>> { .con_id = "foo", id = 15 }, >>> { .con_id = "bar", id = 5 }, >>> }; >>> >>> static struct reset_lookup_table foobar_reset_lookup_table = { >>> .dev_id = "foobar-device", >>> .entries = foobar_reset_lookup_entries, >>> .num_entries = ARRAY_SIZE(foobar_reset_lookup_entries), >>> .dev = &foobar_reset_dev.dev, >>> }; >>> >> >> This seems like a lot of boilerplate to register a lookup. > > This could be shortened a bit by following the gpiod lookup model, > adding a RESET_LOOKUP macro and making the array NULL terminated: > > #define RESET_LOOKUP(reset_dev_id, idx, con_id) /*...*/ > > static struct reset_lookup_table foobar_reset_lookup_table = { > .dev_id = "foobar-device", > .entries = { > RESET_LOOKUP("foobar-reset.0", 15, "foo"), > RESET_LOOKUP("foobar-reset.0", 5, "bar"), > { }, > }, > }; > > /*...*/ > reset_add_lookup_table(&foobar_reset_lookup_table); > >> Can we have >> something like phy_create_lookup() instead where there is just a single >> function call to register a single lookup? This will be much easier to >> use in the davinci PSC driver. >> >> void reset_add_lookup(struct reset_controller_dev *rdev, int index, >> const char *dev_id, const char *con_id); > > In your case the platform code that adds the lookup may be identical to > the code that registers the struct reset_controller_dev, but that > doesn't have to be the case. I'm not sure how that is supposed to work > for the phy framework (I see no platform code adding phy lookups, only > drivers). > > My point was that if the reset controller is registered by a separate > driver, the platform code may not have access to the struct > reset_controller_dev, or even the struct platform_device. I like that > the gpiod lookups can match by dev_id of the gpio chip. > > regards > Philipp >
In our use case, we would be adding the lookup in the driver rather than in the platform code, which is why I am suggesting doing it like the phy framework.
| |