Re: [PATCH v4] reset: add support for non-DT systems

2018-02-23 Thread Bartosz Golaszewski
2018-02-22 17:44 GMT+01:00 David Lechner :
> On 02/22/2018 05:34 AM, Philipp Zabel wrote:
>>
>> On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
>> [...]

 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).

>>> 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.
>>
>>
>> Shouldn't it be the job of the platform code to describe the connections
>> between reset controller and peripheral module reset
>> inputs?
>
>
>
> I guess that depends on who you ask. There are many clock driver that
> register clkdev lookups in drivers/clk/, so that is what we have done
> with the clock driver we are working on. The clock device is also the
> reset controller, so it makes sense to me to register the reset lookup
> in the same place that we are registering the clkdev lookup.
>
> We have a platform_device_id for each possible configuration, so that
> it works very much like the device tree compatible string. You register
> a platform device in the board file with the proper name and the driver
> takes care of the reset because it knows which connections there are
> based on the device name.

I sent another attempt. Since the in-progress psc driver has all the
clocks in the driver code, it makes sense to do the same for resets.

Bart


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-23 Thread Bartosz Golaszewski
2018-02-22 17:44 GMT+01:00 David Lechner :
> On 02/22/2018 05:34 AM, Philipp Zabel wrote:
>>
>> On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
>> [...]

 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).

>>> 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.
>>
>>
>> Shouldn't it be the job of the platform code to describe the connections
>> between reset controller and peripheral module reset
>> inputs?
>
>
>
> I guess that depends on who you ask. There are many clock driver that
> register clkdev lookups in drivers/clk/, so that is what we have done
> with the clock driver we are working on. The clock device is also the
> reset controller, so it makes sense to me to register the reset lookup
> in the same place that we are registering the clkdev lookup.
>
> We have a platform_device_id for each possible configuration, so that
> it works very much like the device tree compatible string. You register
> a platform device in the board file with the proper name and the driver
> takes care of the reset because it knows which connections there are
> based on the device name.

I sent another attempt. Since the in-progress psc driver has all the
clocks in the driver code, it makes sense to do the same for resets.

Bart


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-22 Thread David Lechner

On 02/22/2018 05:34 AM, Philipp Zabel wrote:

On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
[...]

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).


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.


Shouldn't it be the job of the platform code to describe the connections
between reset controller and peripheral module reset
inputs?



I guess that depends on who you ask. There are many clock driver that
register clkdev lookups in drivers/clk/, so that is what we have done
with the clock driver we are working on. The clock device is also the
reset controller, so it makes sense to me to register the reset lookup
in the same place that we are registering the clkdev lookup.

We have a platform_device_id for each possible configuration, so that
it works very much like the device tree compatible string. You register
a platform device in the board file with the proper name and the driver
takes care of the reset because it knows which connections there are
based on the device name.


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-22 Thread David Lechner

On 02/22/2018 05:34 AM, Philipp Zabel wrote:

On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
[...]

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).


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.


Shouldn't it be the job of the platform code to describe the connections
between reset controller and peripheral module reset
inputs?



I guess that depends on who you ask. There are many clock driver that
register clkdev lookups in drivers/clk/, so that is what we have done
with the clock driver we are working on. The clock device is also the
reset controller, so it makes sense to me to register the reset lookup
in the same place that we are registering the clkdev lookup.

We have a platform_device_id for each possible configuration, so that
it works very much like the device tree compatible string. You register
a platform device in the board file with the proper name and the driver
takes care of the reset because it knows which connections there are
based on the device name.


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-22 Thread Bartosz Golaszewski
2018-02-22 12:34 GMT+01:00 Philipp Zabel :
> On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
> [...]
>> > 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).
>> >
>> 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.
>
> Shouldn't it be the job of the platform code to describe the connections
> between reset controller and peripheral module reset
> inputs?
>
> regards
> Philipp

Am I right to understand that it's ok for drivers to know about the
available reset lines on the platform, it's just the associated
between these lines and concerned devices that should be done in
platform code?

Bart


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-22 Thread Bartosz Golaszewski
2018-02-22 12:34 GMT+01:00 Philipp Zabel :
> On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
> [...]
>> > 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).
>> >
>> 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.
>
> Shouldn't it be the job of the platform code to describe the connections
> between reset controller and peripheral module reset
> inputs?
>
> regards
> Philipp

Am I right to understand that it's ok for drivers to know about the
available reset lines on the platform, it's just the associated
between these lines and concerned devices that should be done in
platform code?

Bart


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-22 Thread Philipp Zabel
On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
[...]
> > 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).
> > 
> 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.

Shouldn't it be the job of the platform code to describe the connections
between reset controller and peripheral module reset
inputs?

regards
Philipp


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-22 Thread Philipp Zabel
On Tue, 2018-02-20 at 10:40 -0600, David Lechner wrote:
[...]
> > 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).
> > 
> 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.

Shouldn't it be the job of the platform code to describe the connections
between reset controller and peripheral module reset
inputs?

regards
Philipp


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-20 Thread David Lechner

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 

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 = _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(_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.



Re: [PATCH v4] reset: add support for non-DT systems

2018-02-20 Thread David Lechner

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 

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 = _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(_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.



Re: [PATCH v4] reset: add support for non-DT systems

2018-02-20 Thread Philipp Zabel
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 
> > 
> > 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 = _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(_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


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-20 Thread Philipp Zabel
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 
> > 
> > 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 = _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(_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


Re: [PATCH v4] reset: add support for non-DT systems

2018-02-19 Thread David Lechner

On 02/19/2018 10:58 AM, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

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 = _reset_dev.dev,
};



This seems like a lot of boilerplate to register a lookup. 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);



Re: [PATCH v4] reset: add support for non-DT systems

2018-02-19 Thread David Lechner

On 02/19/2018 10:58 AM, Bartosz Golaszewski wrote:

From: Bartosz Golaszewski 

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 = _reset_dev.dev,
};



This seems like a lot of boilerplate to register a lookup. 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);



[PATCH v4] reset: add support for non-DT systems

2018-02-19 Thread Bartosz Golaszewski
From: Bartosz Golaszewski 

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 = _reset_dev.dev,
};

Cc: Sekhar Nori 
Cc: Kevin Hilman 
Cc: David Lechner 
Signed-off-by: Bartosz Golaszewski 
---
v1 -> v2:
- renamed the new function to __reset_control_get_from_lookup()
- added a missing break; when a matching entry is found
- rearranged the code in __reset_control_get() - we can no longer get to the
  return at the bottom, so remove it and return from
  __reset_control_get_from_lookup() if __of_reset_control_get() fails
- return -ENOENT from reset_contol_get() if we can't find a matching entry,
  prevously returned -EINVAL referred to the fact that we passed a device
  without the of_node which is no longer an error condition
- add a comment about needing a sentinel in the lookup table

v2 -> v3:
- added the reset id number field to the lookup struct so that we don't need
  to rely on the array index

v3 -> v4:
- separated the driver and lookup table registration logic by adding a
  function meant to be called by machine-specific code that adds a lookup
  table to the internal list
- the correct reset controller is now found by first finding the lookup
  table associated with it, then finding the actual reset controller by
  the associated device

 drivers/reset/core.c | 75 +++-
 include/linux/reset-controller.h | 32 +
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index da4292e9de97..fc6abdaf44f2 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -23,6 +23,9 @@
 static DEFINE_MUTEX(reset_list_mutex);
 static LIST_HEAD(reset_controller_list);
 
+static DEFINE_MUTEX(reset_lookup_mutex);
+static LIST_HEAD(reset_lookup_list);
+
 /**
  * struct reset_control - a reset control
  * @rcdev: a pointer to the reset controller device
@@ -493,6 +496,76 @@ struct reset_control *__of_reset_control_get(struct 
device_node *node,
 }
 EXPORT_SYMBOL_GPL(__of_reset_control_get);
 
+/**
+ * reset_add_lookup_table - add a new reset lookup table
+ * @table: new reset lookup table
+ */
+void reset_add_lookup_table(struct reset_lookup_table *table)
+{
+   mutex_lock(_lookup_mutex);
+   list_add_tail(>list, _lookup_list);
+   mutex_unlock(_lookup_mutex);
+}
+EXPORT_SYMBOL_GPL(reset_add_lookup_table);
+
+static struct reset_control *
+reset_control_find_device(struct device *dev, unsigned int id, bool shared)
+{
+   struct reset_controller_dev *rcdev;
+   struct reset_control *rstc = NULL;
+
+   mutex_lock(_list_mutex);
+
+   list_for_each_entry(rcdev, _controller_list, list) {
+   if (rcdev->dev == dev) {
+   rstc = __reset_control_get_internal(rcdev, id, shared);
+   break;
+   }
+   }
+
+   mutex_unlock(_list_mutex);
+
+   return rstc;
+}
+
+static struct reset_control *
+__reset_control_get_from_lookup(struct device *dev, const char *con_id,
+   bool shared, bool optional)
+{
+   const struct reset_lookup_entry *entry;
+   const char *dev_id = dev_name(dev);
+   struct reset_control *rstc = NULL;
+   struct reset_lookup_table *table;
+   int i;
+
+   

[PATCH v4] reset: add support for non-DT systems

2018-02-19 Thread Bartosz Golaszewski
From: Bartosz Golaszewski 

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 = _reset_dev.dev,
};

Cc: Sekhar Nori 
Cc: Kevin Hilman 
Cc: David Lechner 
Signed-off-by: Bartosz Golaszewski 
---
v1 -> v2:
- renamed the new function to __reset_control_get_from_lookup()
- added a missing break; when a matching entry is found
- rearranged the code in __reset_control_get() - we can no longer get to the
  return at the bottom, so remove it and return from
  __reset_control_get_from_lookup() if __of_reset_control_get() fails
- return -ENOENT from reset_contol_get() if we can't find a matching entry,
  prevously returned -EINVAL referred to the fact that we passed a device
  without the of_node which is no longer an error condition
- add a comment about needing a sentinel in the lookup table

v2 -> v3:
- added the reset id number field to the lookup struct so that we don't need
  to rely on the array index

v3 -> v4:
- separated the driver and lookup table registration logic by adding a
  function meant to be called by machine-specific code that adds a lookup
  table to the internal list
- the correct reset controller is now found by first finding the lookup
  table associated with it, then finding the actual reset controller by
  the associated device

 drivers/reset/core.c | 75 +++-
 include/linux/reset-controller.h | 32 +
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index da4292e9de97..fc6abdaf44f2 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -23,6 +23,9 @@
 static DEFINE_MUTEX(reset_list_mutex);
 static LIST_HEAD(reset_controller_list);
 
+static DEFINE_MUTEX(reset_lookup_mutex);
+static LIST_HEAD(reset_lookup_list);
+
 /**
  * struct reset_control - a reset control
  * @rcdev: a pointer to the reset controller device
@@ -493,6 +496,76 @@ struct reset_control *__of_reset_control_get(struct 
device_node *node,
 }
 EXPORT_SYMBOL_GPL(__of_reset_control_get);
 
+/**
+ * reset_add_lookup_table - add a new reset lookup table
+ * @table: new reset lookup table
+ */
+void reset_add_lookup_table(struct reset_lookup_table *table)
+{
+   mutex_lock(_lookup_mutex);
+   list_add_tail(>list, _lookup_list);
+   mutex_unlock(_lookup_mutex);
+}
+EXPORT_SYMBOL_GPL(reset_add_lookup_table);
+
+static struct reset_control *
+reset_control_find_device(struct device *dev, unsigned int id, bool shared)
+{
+   struct reset_controller_dev *rcdev;
+   struct reset_control *rstc = NULL;
+
+   mutex_lock(_list_mutex);
+
+   list_for_each_entry(rcdev, _controller_list, list) {
+   if (rcdev->dev == dev) {
+   rstc = __reset_control_get_internal(rcdev, id, shared);
+   break;
+   }
+   }
+
+   mutex_unlock(_list_mutex);
+
+   return rstc;
+}
+
+static struct reset_control *
+__reset_control_get_from_lookup(struct device *dev, const char *con_id,
+   bool shared, bool optional)
+{
+   const struct reset_lookup_entry *entry;
+   const char *dev_id = dev_name(dev);
+   struct reset_control *rstc = NULL;
+   struct reset_lookup_table *table;
+   int i;
+
+   mutex_lock(_lookup_mutex);
+
+   list_for_each_entry(table, _lookup_list, list) {
+   if (strcmp(table->dev_id,