diff mbox

[v3,06/10] mfd: da9063: Add custom regmap for DA9063L

Message ID 20180602101155.26375-6-marek.vasut+renesas@gmail.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Marek Vasut June 2, 2018, 10:11 a.m. UTC
While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
prevent access into that register block.

Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steve Twiss <stwiss.opensource@diasemi.com>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: linux-renesas-soc@vger.kernel.org
---
V3: New patch
---
 drivers/mfd/da9063-i2c.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

Comments

Geert Uytterhoeven June 4, 2018, 7:39 a.m. UTC | #1
Hi Marek, Steve,

On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
> prevent access into that register block.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Thanks for your patch!

> --- a/drivers/mfd/da9063-i2c.c
> +++ b/drivers/mfd/da9063-i2c.c
> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,

Note that the line above doesn't check da9063->type, but da9063->variant_code...

>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {

... so this may be slightly confusing.

> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>         } else {
>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;

However, da9063->variant_code doesn't seem to have been filled in at this
point yet (the call to da9063_device_init() doing so is below, at the end
of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
support for AD silicon variant") never actually handled the AD silicon variant
correctly? Or am I missing something?

Gr{oetje,eeting}s,

                        Geert
Marek Vasut June 4, 2018, 4:25 p.m. UTC | #2
On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
> Hi Marek, Steve,
> 
> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
>> prevent access into that register block.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
> 
> Thanks for your patch!
> 
>> --- a/drivers/mfd/da9063-i2c.c
>> +++ b/drivers/mfd/da9063-i2c.c
>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
> 
> Note that the line above doesn't check da9063->type, but da9063->variant_code...
> 
>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
> 
> ... so this may be slightly confusing.

I know.

>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>>         } else {
>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
> 
> However, da9063->variant_code doesn't seem to have been filled in at this
> point yet (the call to da9063_device_init() doing so is below, at the end
> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
> support for AD silicon variant") never actually handled the AD silicon variant
> correctly? Or am I missing something?

Ha, that is a good point.
Steve Twiss June 5, 2018, 8:17 p.m. UTC | #3
Hi Marek and Geert,

On 04 June 2018 17:25 Marek Vasut wrote,

> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

> 

> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:

> > Hi Marek, Steve,

> >

> > On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:

> >> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register

> >> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to

> >> prevent access into that register block.


Ok. I've said previously in [v3 07/10], but I'll copy again:
There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
(2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
"Reserved".

Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
Also Dialog do not store a history of Datasheets on their website so once this is updated (although
this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
make the commit message just as misleading as the current datasheet.

How about something simpler?
"The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
into that register block."

> >>

> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

> >

> > Thanks for your patch!

> >

> >> --- a/drivers/mfd/da9063-i2c.c

> >> +++ b/drivers/mfd/da9063-i2c.c

> >> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,

> >

> > Note that the line above doesn't check da9063->type, but da9063-

> >variant_code...

> >

> >>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;

> >>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;

> >>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;

> >> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {

> >

> > ... so this may be slightly confusing.

> 

> I know.

> 

> >> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;

> >> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;

> >> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;

> >>         } else {

> >>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;

> >>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;

> >

> > However, da9063->variant_code doesn't seem to have been filled in at this

> > point yet (the call to da9063_device_init() doing so is below, at the end

> > of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add

> > support for AD silicon variant") never actually handled the AD silicon variant

> > correctly? Or am I missing something?


Okay ... No. You're not missing anything. I had noticed that.
The AD chip model is not referenced and by default only the BB chip model is used.

> Ha, that is a good point.


Yeah, it's a good point, but it's not an amusing point.
The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
There is no datasheet listing AD registers supported by Dialog, only BB.

But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
and the RTC driver was updated to distinguish between the AD and BB according to
the type of variant detected at run-time during the da9063_device_init() call.

The real problem is that this leads to two competing chip detection methods for the
DA9063. The function da9063_device_init() autodetects the chip variant, but
autodetection cannot define the chip model. It's circular: the chip model cannot be
autodetected because a chip model is needed to access the register used during
autodetection.

Which leads me back to what I said two paragraphs up:
> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.

> There is no datasheet listing AD registers supported by Dialog, only BB.


This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
used to print the information to the console during start-up and it is the DT that defines
the chip model based upon "dlg,da9062" or "dlg,da9061".

Steve
Marek Vasut June 5, 2018, 11:02 p.m. UTC | #4
On 06/05/2018 10:17 PM, Steve Twiss wrote:
> Hi Marek and Geert,
> 
> On 04 June 2018 17:25 Marek Vasut wrote,
> 
>> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
>>
>> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
>>> Hi Marek, Steve,
>>>
>>> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>>>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
>>>> prevent access into that register block.
> 
> Ok. I've said previously in [v3 07/10], but I'll copy again:
> There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
> Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
> (2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
> on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
> "Reserved".
> 
> Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
> Also Dialog do not store a history of Datasheets on their website so once this is updated (although
> this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
> make the commit message just as misleading as the current datasheet.
> 
> How about something simpler?
> "The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
> into that register block."
> 
>>>>
>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>
>>> Thanks for your patch!
>>>
>>>> --- a/drivers/mfd/da9063-i2c.c
>>>> +++ b/drivers/mfd/da9063-i2c.c
>>>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
>>>
>>> Note that the line above doesn't check da9063->type, but da9063-
>>> variant_code...
>>>
>>>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>>>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>>>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
>>>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
>>>
>>> ... so this may be slightly confusing.
>>
>> I know.
>>
>>>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
>>>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
>>>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>>>>         } else {
>>>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>>>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
>>>
>>> However, da9063->variant_code doesn't seem to have been filled in at this
>>> point yet (the call to da9063_device_init() doing so is below, at the end
>>> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
>>> support for AD silicon variant") never actually handled the AD silicon variant
>>> correctly? Or am I missing something?
> 
> Okay ... No. You're not missing anything. I had noticed that.
> The AD chip model is not referenced and by default only the BB chip model is used.
> 
>> Ha, that is a good point.
> 
> Yeah, it's a good point, but it's not an amusing point.
> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
> There is no datasheet listing AD registers supported by Dialog, only BB.
> 
> But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
> and the RTC driver was updated to distinguish between the AD and BB according to
> the type of variant detected at run-time during the da9063_device_init() call.
> 
> The real problem is that this leads to two competing chip detection methods for the
> DA9063. The function da9063_device_init() autodetects the chip variant, but
> autodetection cannot define the chip model. It's circular: the chip model cannot be
> autodetected because a chip model is needed to access the register used during
> autodetection.
> 
> Which leads me back to what I said two paragraphs up:
>> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
>> There is no datasheet listing AD registers supported by Dialog, only BB.
> 
> This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
> used to print the information to the console during start-up and it is the DT that defines
> the chip model based upon "dlg,da9062" or "dlg,da9061".

So the AD was broken since forever and noone noticed ? :)

Do you have an AD hardware and can you fix it ?
Steve Twiss June 6, 2018, 9:47 a.m. UTC | #5
Hi Marek and Geert,

On 06 June 2018 00:02 Marek Vasut wrote,

> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

> 

> On 06/05/2018 10:17 PM, Steve Twiss wrote:

> > Hi Marek and Geert,

> >

> > On 04 June 2018 17:25 Marek Vasut wrote,

> >

> >> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L

> >>

> >> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:

> >>> Hi Marek, Steve,

> >>>

> >>> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:

> >>>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register

> >>>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to

> >>>> prevent access into that register block.

> >

> > Ok. I've said previously in [v3 07/10], but I'll copy again:

> > There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.

> > Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L

> > (2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table

> > on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as

> > "Reserved".

> >

> > Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.

> > Also Dialog do not store a history of Datasheets on their website so once this is updated (although

> > this update is not in my hands) the datasheet will be replaced. So, it seems this comment could

> > make the commit message just as misleading as the current datasheet.

> >

> > How about something simpler?

> > "The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access

> > into that register block."

> >

> >>>>

> >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

> >>>

> >>> Thanks for your patch!

> >>>

> >>>> --- a/drivers/mfd/da9063-i2c.c

> >>>> +++ b/drivers/mfd/da9063-i2c.c

> >>>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,

> >>>

> >>> Note that the line above doesn't check da9063->type, but da9063-

> >>> variant_code...

> >>>

> >>>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;

> >>>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;

> >>>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;

> >>>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {

> >>>

> >>> ... so this may be slightly confusing.

> >>

> >> I know.

> >>

> >>>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;

> >>>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;

> >>>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;

> >>>>         } else {

> >>>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;

> >>>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;

> >>>

> >>> However, da9063->variant_code doesn't seem to have been filled in at this

> >>> point yet (the call to da9063_device_init() doing so is below, at the end

> >>> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add

> >>> support for AD silicon variant") never actually handled the AD silicon variant

> >>> correctly? Or am I missing something?

> >

> > Okay ... No. You're not missing anything. I had noticed that.

> > The AD chip model is not referenced and by default only the BB chip model is used.

> >

> >> Ha, that is a good point.

> >

> > Yeah, it's a good point, but it's not an amusing point.

> > The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.

> > There is no datasheet listing AD registers supported by Dialog, only BB.

> >

> > But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91

> > and the RTC driver was updated to distinguish between the AD and BB according to

> > the type of variant detected at run-time during the da9063_device_init() call.

> >

> > The real problem is that this leads to two competing chip detection methods for the

> > DA9063. The function da9063_device_init() autodetects the chip variant, but

> > autodetection cannot define the chip model. It's circular: the chip model cannot be

> > autodetected because a chip model is needed to access the register used during

> > autodetection.

> >

> > Which leads me back to what I said two paragraphs up:

> >> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.

> >> There is no datasheet listing AD registers supported by Dialog, only BB.

> >

> > This is not how it is done in the DA9062 and DA9061 driver: the variant code is only

> > used to print the information to the console during start-up and it is the DT that defines

> > the chip model based upon "dlg,da9062" or "dlg,da9061".

> 

> So the AD was broken since forever and noone noticed ? :)


Not quite. 
The AD support is working, but the driver doesn't work like everybody
expects because it uses the BB chip model. But it does work because the chip
model for BB is valid for AD; in this case BB represents a superset of AD
registers (and any mismatches are never accessed or mean anything in AD).

> Do you have an AD hardware and can you fix it ?


Part of my work is to support the community and I think this is fixable.

But all of this shouldn't affect your DA9063L submission should it?

Regards,
Steve
Marek Vasut June 6, 2018, 9:50 a.m. UTC | #6
On 06/06/2018 11:47 AM, Steve Twiss wrote:
> Hi Marek and Geert,
> 
> On 06 June 2018 00:02 Marek Vasut wrote,
> 
>> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
>>
>> On 06/05/2018 10:17 PM, Steve Twiss wrote:
>>> Hi Marek and Geert,
>>>
>>> On 04 June 2018 17:25 Marek Vasut wrote,
>>>
>>>> Subject: Re: [PATCH v3 06/10] mfd: da9063: Add custom regmap for DA9063L
>>>>
>>>> On 06/04/2018 09:39 AM, Geert Uytterhoeven wrote:
>>>>> Hi Marek, Steve,
>>>>>
>>>>> On Sat, Jun 2, 2018 at 12:11 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>>>> While the datasheet for DA9063L (2v1, 23-Mar-2017) lists the RTC register
>>>>>> block, the DA9063L does not have an RTC. Add custom regmap for DA9063L to
>>>>>> prevent access into that register block.
>>>
>>> Ok. I've said previously in [v3 07/10], but I'll copy again:
>>> There is now an internal Dialog request to remove the RTC references from the DA9063L datasheet.
>>> Adding that first part to the sentence in the commit log: "While the datasheet for DA9063L
>>> (2v1, 23-Mar-2017) lists the RTC register block" -- it exists in error for the register map table
>>> on page 91, but the datasheet also identifies those registers in Table 102 on page 126 as
>>> "Reserved".
>>>
>>> Pointing out the ambiguity in this version of the datasheet seems redundant in the commit log.
>>> Also Dialog do not store a history of Datasheets on their website so once this is updated (although
>>> this update is not in my hands) the datasheet will be replaced. So, it seems this comment could
>>> make the commit message just as misleading as the current datasheet.
>>>
>>> How about something simpler?
>>> "The DA9063L does not have an RTC. Add custom regmap for DA9063L to prevent access
>>> into that register block."
>>>
>>>>>>
>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- a/drivers/mfd/da9063-i2c.c
>>>>>> +++ b/drivers/mfd/da9063-i2c.c
>>>>>> @@ -254,6 +341,10 @@ static int da9063_i2c_probe(struct i2c_client *i2c,
>>>>>
>>>>> Note that the line above doesn't check da9063->type, but da9063-
>>>>> variant_code...
>>>>>
>>>>>>                 da9063_regmap_config.rd_table = &da9063_ad_readable_table;
>>>>>>                 da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
>>>>>>                 da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
>>>>>> +       } else if (da9063->type == PMIC_TYPE_DA9063L) {
>>>>>
>>>>> ... so this may be slightly confusing.
>>>>
>>>> I know.
>>>>
>>>>>> +               da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
>>>>>> +               da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
>>>>>> +               da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
>>>>>>         } else {
>>>>>>                 da9063_regmap_config.rd_table = &da9063_bb_readable_table;
>>>>>>                 da9063_regmap_config.wr_table = &da9063_bb_writeable_table;
>>>>>
>>>>> However, da9063->variant_code doesn't seem to have been filled in at this
>>>>> point yet (the call to da9063_device_init() doing so is below, at the end
>>>>> of the probe function!), so commit 9cb42e2a8ed06e91 ("mfd: da9063: Add
>>>>> support for AD silicon variant") never actually handled the AD silicon variant
>>>>> correctly? Or am I missing something?
>>>
>>> Okay ... No. You're not missing anything. I had noticed that.
>>> The AD chip model is not referenced and by default only the BB chip model is used.
>>>
>>>> Ha, that is a good point.
>>>
>>> Yeah, it's a good point, but it's not an amusing point.
>>> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
>>> There is no datasheet listing AD registers supported by Dialog, only BB.
>>>
>>> But, AD registers were added back into the header file in commit 9cb42e2a8ed06e91
>>> and the RTC driver was updated to distinguish between the AD and BB according to
>>> the type of variant detected at run-time during the da9063_device_init() call.
>>>
>>> The real problem is that this leads to two competing chip detection methods for the
>>> DA9063. The function da9063_device_init() autodetects the chip variant, but
>>> autodetection cannot define the chip model. It's circular: the chip model cannot be
>>> autodetected because a chip model is needed to access the register used during
>>> autodetection.
>>>
>>> Which leads me back to what I said two paragraphs up:
>>>> The device tree only distinguishes a "dlg,da9063", there is no AD type in the DT schema.
>>>> There is no datasheet listing AD registers supported by Dialog, only BB.
>>>
>>> This is not how it is done in the DA9062 and DA9061 driver: the variant code is only
>>> used to print the information to the console during start-up and it is the DT that defines
>>> the chip model based upon "dlg,da9062" or "dlg,da9061".
>>
>> So the AD was broken since forever and noone noticed ? :)
> 
> Not quite. 
> The AD support is working, but the driver doesn't work like everybody
> expects because it uses the BB chip model. But it does work because the chip
> model for BB is valid for AD; in this case BB represents a superset of AD
> registers (and any mismatches are never accessed or mean anything in AD).
> 
>> Do you have an AD hardware and can you fix it ?
> 
> Part of my work is to support the community and I think this is fixable.
> 
> But all of this shouldn't affect your DA9063L submission should it?

I think there might be conflict between those patchsets, so let me send
out a V5 so you can play around with the AD and fix that too.
diff mbox

Patch

diff --git a/drivers/mfd/da9063-i2c.c b/drivers/mfd/da9063-i2c.c
index 048ce55ebc5b..f0d92a37df6b 100644
--- a/drivers/mfd/da9063-i2c.c
+++ b/drivers/mfd/da9063-i2c.c
@@ -208,6 +208,93 @@  static const struct regmap_access_table da9063_bb_volatile_table = {
 	.n_yes_ranges = ARRAY_SIZE(da9063_bb_volatile_ranges),
 };
 
+static const struct regmap_range da9063l_bb_readable_ranges[] = {
+	{
+		.range_min = DA9063_REG_PAGE_CON,
+		.range_max = DA9063_REG_MON_A10_RES,
+	}, {
+		.range_min = DA9063_REG_SEQ,
+		.range_max = DA9063_REG_ID_32_31,
+	}, {
+		.range_min = DA9063_REG_SEQ_A,
+		.range_max = DA9063_REG_AUTO3_LOW,
+	}, {
+		.range_min = DA9063_REG_T_OFFSET,
+		.range_max = DA9063_BB_REG_GP_ID_19,
+	}, {
+		.range_min = DA9063_REG_CHIP_ID,
+		.range_max = DA9063_REG_CHIP_VARIANT,
+	},
+};
+
+static const struct regmap_range da9063l_bb_writeable_ranges[] = {
+	{
+		.range_min = DA9063_REG_PAGE_CON,
+		.range_max = DA9063_REG_PAGE_CON,
+	}, {
+		.range_min = DA9063_REG_FAULT_LOG,
+		.range_max = DA9063_REG_VSYS_MON,
+	}, {
+		.range_min = DA9063_REG_SEQ,
+		.range_max = DA9063_REG_ID_32_31,
+	}, {
+		.range_min = DA9063_REG_SEQ_A,
+		.range_max = DA9063_REG_AUTO3_LOW,
+	}, {
+		.range_min = DA9063_REG_CONFIG_I,
+		.range_max = DA9063_BB_REG_MON_REG_4,
+	}, {
+		.range_min = DA9063_BB_REG_GP_ID_0,
+		.range_max = DA9063_BB_REG_GP_ID_19,
+	},
+};
+
+static const struct regmap_range da9063l_bb_volatile_ranges[] = {
+	{
+		.range_min = DA9063_REG_PAGE_CON,
+		.range_max = DA9063_REG_EVENT_D,
+	}, {
+		.range_min = DA9063_REG_CONTROL_A,
+		.range_max = DA9063_REG_CONTROL_B,
+	}, {
+		.range_min = DA9063_REG_CONTROL_E,
+		.range_max = DA9063_REG_CONTROL_F,
+	}, {
+		.range_min = DA9063_REG_BCORE2_CONT,
+		.range_max = DA9063_REG_LDO11_CONT,
+	}, {
+		.range_min = DA9063_REG_DVC_1,
+		.range_max = DA9063_REG_ADC_MAN,
+	}, {
+		.range_min = DA9063_REG_ADC_RES_L,
+		.range_max = DA9063_REG_MON_A10_RES,
+	}, {
+		.range_min = DA9063_REG_SEQ,
+		.range_max = DA9063_REG_SEQ,
+	}, {
+		.range_min = DA9063_REG_EN_32K,
+		.range_max = DA9063_REG_EN_32K,
+	}, {
+		.range_min = DA9063_BB_REG_MON_REG_5,
+		.range_max = DA9063_BB_REG_MON_REG_6,
+	},
+};
+
+static const struct regmap_access_table da9063l_bb_readable_table = {
+	.yes_ranges = da9063l_bb_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9063l_bb_readable_ranges),
+};
+
+static const struct regmap_access_table da9063l_bb_writeable_table = {
+	.yes_ranges = da9063l_bb_writeable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9063l_bb_writeable_ranges),
+};
+
+static const struct regmap_access_table da9063l_bb_volatile_table = {
+	.yes_ranges = da9063l_bb_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(da9063l_bb_volatile_ranges),
+};
+
 static const struct regmap_range_cfg da9063_range_cfg[] = {
 	{
 		.range_min = DA9063_REG_PAGE_CON,
@@ -254,6 +341,10 @@  static int da9063_i2c_probe(struct i2c_client *i2c,
 		da9063_regmap_config.rd_table = &da9063_ad_readable_table;
 		da9063_regmap_config.wr_table = &da9063_ad_writeable_table;
 		da9063_regmap_config.volatile_table = &da9063_ad_volatile_table;
+	} else if (da9063->type == PMIC_TYPE_DA9063L) {
+		da9063_regmap_config.rd_table = &da9063l_bb_readable_table;
+		da9063_regmap_config.wr_table = &da9063l_bb_writeable_table;
+		da9063_regmap_config.volatile_table = &da9063l_bb_volatile_table;
 	} else {
 		da9063_regmap_config.rd_table = &da9063_bb_readable_table;
 		da9063_regmap_config.wr_table = &da9063_bb_writeable_table;