[PATCH v2 01/29] nvmem: add support for cell lookups

On 28/08/18 15:41, Bartosz Golaszewski wrote:

2018-08-28 15:45 GMT+02:00 Srinivas Kandagatla [off-list ref]:

quoted

...

quoted

quoted

I would like to support an additional use case here: the provider is
generic and is not aware of its cells at all. Since the only way of
defining nvmem cells is through DT or nvmem_config, we lack a way to
allow machine code to define cells without the provider code being
aware.

machine driver should be able to do
nvmem_device_get()
nvmem_add_cells()

Indeed, I missed the fact that you can retrieve the nvmem device by
name. Except that we cannot know that the nvmem provider has been
registered yet when calling nvmem_device_get(). This could potentially
be solved by my other patch that adds notifiers to nvmem, but it would
require much more boilerplate code in every board file. I think that
removing nvmem_cell_info from nvmem_config and having external cell
definitions would be cleaner.

Yes, notifiers would work!

...

quoted

quoted

Yes, I would like to rework nvmem a bit. I don't see any non-DT users
defining nvmem-cells using nvmem_config. I think that what we need is
a way of specifying cell config outside of nvmem providers in some
kind of structures. These tables would reference the provider by name
and define the cells. Then we would have an additional lookup
structure which would associate the consumer (by dev_id and con_id,
where dev_id could optionally be NULL and where we would fall back to
using con_id only) and the nvmem provider + cell together. Similarly
to how GPIO consumers are associated with the gpiochip and hwnum. How
does it sound?

Yes, sounds good.

Correct me if am wrong!
You should be able to add the new cells using struct nvmem_cell_info and add
them to particular provider using nvmem_add_cells().

Sounds like thats exactly what nvmem_add_lookup_table() would look like.

We should add new nvmem_device_cell_get(nvmem, conn_id) which would return
nvmem cell which is specific to the provider. This cell can be used by the
machine driver to read/write.

Except that we could do it lazily - when the nvmem provider actually
gets registered instead of doing it right away and risking that the
device isn't even there yet.

Yes, it makes more sense to do it once the provider is actually present!

quoted

quoted

quoted

quoted

BTW: of_nvmem_cell_get() seems to always allocate an nvmem_cell
instance even if the cell for this node was already added to the nvmem
device.

I hope you got the reason why of_nvmem_cell_get() always allocates new
instance for every get!!

I admit I didn't test it, but just from reading the code it seems like
in nvmem_cell_get() for DT-users we'll always get to
of_nvmem_cell_get() and in there we always end up calling line 873:
cell = kzalloc(sizeof(*cell), GFP_KERNEL);

That is correct, this cell is created when we do a get and release when we
do a put().

Shouldn't we add the cell to the list, and check first if it's there
and only create it if not?

Yes I agree, duplicate entry checks are missing!

--srini

Bart