2017-12-02 12:10:09

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] mfd: madera: Add common support for Cirrus Logic Madera codecs

On Wed, Nov 29, 2017 at 12:36 PM, Richard Fitzgerald
<[email protected]> wrote:
> On 29/11/17 10:18, Linus Walleij wrote:
>>
>> On Thu, Nov 23, 2017 at 6:13 PM, Richard Fitzgerald
>> <[email protected]> wrote:
>>
>>> +config MFD_MADERA_I2C
>>> + bool "Cirrus Logic Madera codecs with I2C"
>>> + select MFD_MADERA
>>> + select REGMAP_I2C
>>> + depends on I2C
>>> + depends on PINCTRL
>>> + help
>>> + Support for the Cirrus Logic Madera platform audio SoC
>>> + core functionality controlled via I2C.
>>> +
>>> +config MFD_MADERA_SPI
>>> + bool "Cirrus Logic Madera codecs with SPI"
>>> + select MFD_MADERA
>>> + select REGMAP_SPI
>>> + depends on SPI_MASTER
>>> + depends on PINCTRL
>>> + help
>>> + Support for the Cirrus Logic Madera platform audio SoC
>>> + core functionality controlled via SPI.
>>
>>
>> Why do the I2C and SPI subdrivers depend on PINCTRL?
>>
>> They sure don't seem to be using any pinctrl-specific APIs.
>>
>
> They require PINCTRL even if they don't call any functions on it because the
> chip won't work correctly if there isn't a PINCTRL driver to apply the
> correct pinmux configuration.

Apply the configuration to what? Sorry I don't get it.

You can't be referring to the internal pin controller of the Madera, since
that has to come up before its pin controller can even be communicated
with.

If you mean it is to apply the configuration to the system SoC where
this coded is connected, this is wrong. There may very well be systems
which have dedicated pins for the codec, atleast in theory. The fact
that your reference board needs this is not a universal requirement,
it should be set up in the machine-specific Kconfig for the reference
board in that case.

Yours,
Linus Walleij


2017-12-04 09:47:32

by Richard Fitzgerald

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] mfd: madera: Add common support for Cirrus Logic Madera codecs

On 02/12/17 12:10, Linus Walleij wrote:
> On Wed, Nov 29, 2017 at 12:36 PM, Richard Fitzgerald
> <[email protected]> wrote:
>> On 29/11/17 10:18, Linus Walleij wrote:
>>>
>>> On Thu, Nov 23, 2017 at 6:13 PM, Richard Fitzgerald
>>> <[email protected]> wrote:
>>>
>>>> +config MFD_MADERA_I2C
>>>> + bool "Cirrus Logic Madera codecs with I2C"
>>>> + select MFD_MADERA
>>>> + select REGMAP_I2C
>>>> + depends on I2C
>>>> + depends on PINCTRL
>>>> + help
>>>> + Support for the Cirrus Logic Madera platform audio SoC
>>>> + core functionality controlled via I2C.
>>>> +
>>>> +config MFD_MADERA_SPI
>>>> + bool "Cirrus Logic Madera codecs with SPI"
>>>> + select MFD_MADERA
>>>> + select REGMAP_SPI
>>>> + depends on SPI_MASTER
>>>> + depends on PINCTRL
>>>> + help
>>>> + Support for the Cirrus Logic Madera platform audio SoC
>>>> + core functionality controlled via SPI.
>>>
>>>
>>> Why do the I2C and SPI subdrivers depend on PINCTRL?
>>>
>>> They sure don't seem to be using any pinctrl-specific APIs.
>>>
>>
>> They require PINCTRL even if they don't call any functions on it because the
>> chip won't work correctly if there isn't a PINCTRL driver to apply the
>> correct pinmux configuration.
>
> Apply the configuration to what? Sorry I don't get it.
>
> You can't be referring to the internal pin controller of the Madera, since

Yes I am

> that has to come up before its pin controller can even be communicated
> with.

So?

The MFD driver powers up the chip before registering child drivers.

Also that's not entirely relevant, the pinctrl settings can still be
written with the chip off because they will go into the regmap cache and
be applied when the chip is next resumed.

>
> If you mean it is to apply the configuration to the system SoC where
> this coded is connected, this is wrong. There may very well be systems
> which have dedicated pins for the codec, atleast in theory. The fact
> that your reference board needs this is not a universal requirement,
> it should be set up in the machine-specific Kconfig for the reference
> board in that case.
>
> Yours,
> Linus Walleij
>

2017-12-07 08:54:10

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] mfd: madera: Add common support for Cirrus Logic Madera codecs

On Mon, Dec 4, 2017 at 10:47 AM, Richard Fitzgerald
<[email protected]> wrote:
> On 02/12/17 12:10, Linus Walleij wrote:
>> On Wed, Nov 29, 2017 at 12:36 PM, Richard Fitzgerald
>> <[email protected]> wrote:
>>> On 29/11/17 10:18, Linus Walleij wrote:
>>>> On Thu, Nov 23, 2017 at 6:13 PM, Richard Fitzgerald
>>>> <[email protected]> wrote:
>>>>
>>>>> +config MFD_MADERA_I2C
>>>>> + bool "Cirrus Logic Madera codecs with I2C"
>>>>> + select MFD_MADERA
>>>>> + select REGMAP_I2C
>>>>> + depends on I2C
>>>>> + depends on PINCTRL
>>>>> + help
>>>>> + Support for the Cirrus Logic Madera platform audio SoC
>>>>> + core functionality controlled via I2C.
>>>>> +
>>>>> +config MFD_MADERA_SPI
>>>>> + bool "Cirrus Logic Madera codecs with SPI"
>>>>> + select MFD_MADERA
>>>>> + select REGMAP_SPI
>>>>> + depends on SPI_MASTER
>>>>> + depends on PINCTRL
>>>>> + help
>>>>> + Support for the Cirrus Logic Madera platform audio SoC
>>>>> + core functionality controlled via SPI.
>>>>
>>>>
>>>>
>>>> Why do the I2C and SPI subdrivers depend on PINCTRL?
>>>>
>>>> They sure don't seem to be using any pinctrl-specific APIs.
>>>>
>>>
>>> They require PINCTRL even if they don't call any functions on it because
>>> the
>>> chip won't work correctly if there isn't a PINCTRL driver to apply the
>>> correct pinmux configuration.
>>
>>
>> Apply the configuration to what? Sorry I don't get it.
>>
>> You can't be referring to the internal pin controller of the Madera, since
>
> Yes I am

You are saying that the I2C and SPI interface to the Madera codec
depends on pin control.

It does not.

You can most certainly talk I2C and SPI to the coded without any
pin control. Probably the MFD driver can come up without it.

If what you want is unconditional pin control enabled for this circuit,
then have MFD_MADERA select PINCTRL.

>> that has to come up before its pin controller can even be communicated
>> with.
>
>
> So?
>
> The MFD driver powers up the chip before registering child drivers.

Including the pin controller. You just confirmed what I said: the
I2C and SPI interfaces do not require pin control to talk to the
chip.

> Also that's not entirely relevant, the pinctrl settings can still be written
> with the chip off because they will go into the regmap cache and be applied
> when the chip is next resumed.

You still have the dependencies wrong.

Yours,
Linus Walleij

2017-12-07 10:53:08

by Richard Fitzgerald

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] mfd: madera: Add common support for Cirrus Logic Madera codecs

On 07/12/17 08:54, Linus Walleij wrote:
> On Mon, Dec 4, 2017 at 10:47 AM, Richard Fitzgerald
> <[email protected]> wrote:
>> On 02/12/17 12:10, Linus Walleij wrote:
>>> On Wed, Nov 29, 2017 at 12:36 PM, Richard Fitzgerald
>>> <[email protected]> wrote:
>>>> On 29/11/17 10:18, Linus Walleij wrote:
>>>>> On Thu, Nov 23, 2017 at 6:13 PM, Richard Fitzgerald
>>>>> <[email protected]> wrote:
>>>>>
>>>>>> +config MFD_MADERA_I2C
>>>>>> + bool "Cirrus Logic Madera codecs with I2C"
>>>>>> + select MFD_MADERA
>>>>>> + select REGMAP_I2C
>>>>>> + depends on I2C
>>>>>> + depends on PINCTRL
>>>>>> + help
>>>>>> + Support for the Cirrus Logic Madera platform audio SoC
>>>>>> + core functionality controlled via I2C.
>>>>>> +
>>>>>> +config MFD_MADERA_SPI
>>>>>> + bool "Cirrus Logic Madera codecs with SPI"
>>>>>> + select MFD_MADERA
>>>>>> + select REGMAP_SPI
>>>>>> + depends on SPI_MASTER
>>>>>> + depends on PINCTRL
>>>>>> + help
>>>>>> + Support for the Cirrus Logic Madera platform audio SoC
>>>>>> + core functionality controlled via SPI.
>>>>>
>>>>>
>>>>>
>>>>> Why do the I2C and SPI subdrivers depend on PINCTRL?
>>>>>
>>>>> They sure don't seem to be using any pinctrl-specific APIs.
>>>>>
>>>>
>>>> They require PINCTRL even if they don't call any functions on it because
>>>> the
>>>> chip won't work correctly if there isn't a PINCTRL driver to apply the
>>>> correct pinmux configuration.
>>>
>>>
>>> Apply the configuration to what? Sorry I don't get it.
>>>
>>> You can't be referring to the internal pin controller of the Madera, since
>>
>> Yes I am
>
> You are saying that the I2C and SPI interface to the Madera codec
> depends on pin control.
>
> It does not.
>
> You can most certainly talk I2C and SPI to the coded without any
> pin control. Probably the MFD driver can come up without it.
>
> If what you want is unconditional pin control enabled for this circuit,
> then have MFD_MADERA select PINCTRL.
>
>>> that has to come up before its pin controller can even be communicated
>>> with.
>>
>>
>> So?
>>
>> The MFD driver powers up the chip before registering child drivers.
>
> Including the pin controller. You just confirmed what I said: the
> I2C and SPI interfaces do not require pin control to talk to the
> chip.
>
>> Also that's not entirely relevant, the pinctrl settings can still be written
>> with the chip off because they will go into the regmap cache and be applied
>> when the chip is next resumed.
>
> You still have the dependencies wrong.
>
> Yours,
> Linus Walleij
>

So the short version of this email thread is that it should be "select
PINCTRL" ?

2017-12-12 23:51:18

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] mfd: madera: Add common support for Cirrus Logic Madera codecs

On Thu, Dec 7, 2017 at 11:52 AM, Richard Fitzgerald
<[email protected]> wrote:
> On 07/12/17 08:54, Linus Walleij wrote:

>> If what you want is unconditional pin control enabled for this circuit,
>> then have MFD_MADERA select PINCTRL.

> So the short version of this email thread is that it should be "select
> PINCTRL" ?

If by "it" you mean MFD_MADERA tne I guess yes :D

I was just confused because I couldn't figure out what you were
trying to do.

Yours,
Linus Walleij