[1/2] dt-bindings: i2c: i2c-mt65xx: add additional clocks

Add pck and mck clocks which are needed to access I2C registers on MT7981.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml | 2 ++
 1 file changed, 2 insertions(+)

Il 31/05/23 18:11, Daniel Golle ha scritto:
> On MT7981 additional clocks are required when accessing I2C registers.
> Add MCK and PCK optional clocks to i2c-mt65xx driver so we don't have
> to always have them enabled, but really only if I2C is used.
> 
> Fixes: f82fd1845d309 ("i2c: mediatek: add support for MT7981 SoC")
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>

PCK and MCK should really be P=PMIC and M=MEM as far as I understand, which
means that they should effectively be CLK_PMIC and CLK_ARB.

In that case, you don't need to add new clocks to the driver, as you're using
the very same ones that the driver supports.

Cheers,
Angelo

> ---
>   drivers/i2c/busses/i2c-mt65xx.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index a43c4d77739ab..cd28cbec2b96d 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -93,6 +93,8 @@
>    * @I2C_MT65XX_CLK_DMA:  DMA clock for i2c via DMA
>    * @I2C_MT65XX_CLK_PMIC: PMIC clock for i2c from PMIC
>    * @I2C_MT65XX_CLK_ARB:  Arbitrator clock for i2c
> + * @I2C_MT65XX_CLK_MCK:  MCK clock for i2c
> + * @I2C_MT65XX_CLK_PCK:  PCK clock for i2c
>    * @I2C_MT65XX_CLK_MAX:  Number of supported clocks
>    */
>   enum i2c_mt65xx_clks {
> @@ -100,11 +102,13 @@ enum i2c_mt65xx_clks {
>   	I2C_MT65XX_CLK_DMA,
>   	I2C_MT65XX_CLK_PMIC,
>   	I2C_MT65XX_CLK_ARB,
> +	I2C_MT65XX_CLK_MCK,
> +	I2C_MT65XX_CLK_PCK,
>   	I2C_MT65XX_CLK_MAX
>   };
>   
>   static const char * const i2c_mt65xx_clk_ids[I2C_MT65XX_CLK_MAX] = {
> -	"main", "dma", "pmic", "arb"
> +	"main", "dma", "pmic", "arb", "mck", "pck"
>   };
>   
>   enum DMA_REGS_OFFSET {
> @@ -1444,6 +1448,14 @@ static int mtk_i2c_probe(struct platform_device *pdev)
>   	if (IS_ERR(i2c->clocks[I2C_MT65XX_CLK_ARB].clk))
>   		return PTR_ERR(i2c->clocks[I2C_MT65XX_CLK_ARB].clk);
>   
> +	i2c->clocks[I2C_MT65XX_CLK_MCK].clk = devm_clk_get_optional(&pdev->dev, "mck");
> +	if (IS_ERR(i2c->clocks[I2C_MT65XX_CLK_MCK].clk))
> +		return PTR_ERR(i2c->clocks[I2C_MT65XX_CLK_MCK].clk);
> +
> +	i2c->clocks[I2C_MT65XX_CLK_PCK].clk = devm_clk_get_optional(&pdev->dev, "pck");
> +	if (IS_ERR(i2c->clocks[I2C_MT65XX_CLK_PCK].clk))
> +		return PTR_ERR(i2c->clocks[I2C_MT65XX_CLK_PCK].clk);
> +
>   	if (i2c->have_pmic) {
>   		i2c->clocks[I2C_MT65XX_CLK_PMIC].clk = devm_clk_get(&pdev->dev, "pmic");
>   		if (IS_ERR(i2c->clocks[I2C_MT65XX_CLK_PMIC].clk)) {

On Thu, Jun 01, 2023 at 06:54:01PM +0200, Krzysztof Kozlowski wrote:
> On 31/05/2023 18:10, Daniel Golle wrote:
> > Add pck and mck clocks which are needed to access I2C registers on MT7981.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml
> > index fda0467cdd954..550795f6573c5 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml
> > @@ -78,6 +78,8 @@ properties:
> >        - const: dma
> >        - const: arb
> >        - const: pmic
> > +      - const: mck
> > +      - const: pck
> >  
> 
> Adding names does not magically add the clocks. This wasn't tested.

Adding the clocks is done in patch 2/2 which just wasn't sent to
devicetree@ and dt maintainers, but to the relevant mailing lists
instead. Was that wrong and should I always send the complete series
also to devicetree@ as well as dt maintainers?

Anyway. It turns out that arb and pmic clocks are intended for that,
so we can cancel this and I'll just use those instead.

On Thu, Jun 01, 2023 at 08:10:11PM +0100, Daniel Golle wrote:

> Adding the clocks is done in patch 2/2 which just wasn't sent to
> devicetree@ and dt maintainers, but to the relevant mailing lists
> instead. Was that wrong and should I always send the complete series
> also to devicetree@ as well as dt maintainers?

Everyone is different, but getting both patches of a 2 patch series
really should not annoy anyone and avoiding reviewers having to go
hunting on lore etc is always a positive :)

On 01/06/2023 21:10, Daniel Golle wrote:
> On Thu, Jun 01, 2023 at 06:54:01PM +0200, Krzysztof Kozlowski wrote:
>> On 31/05/2023 18:10, Daniel Golle wrote:
>>> Add pck and mck clocks which are needed to access I2C registers on MT7981.
>>>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>>  Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml
>>> index fda0467cdd954..550795f6573c5 100644
>>> --- a/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml
>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt65xx.yaml
>>> @@ -78,6 +78,8 @@ properties:
>>>        - const: dma
>>>        - const: arb
>>>        - const: pmic
>>> +      - const: mck
>>> +      - const: pck
>>>  
>>
>> Adding names does not magically add the clocks. This wasn't tested.
> 
> Adding the clocks is done in patch 2/2 which just wasn't sent to
> devicetree@ and dt maintainers, but to the relevant mailing lists
> instead. Was that wrong and should I always send the complete series
> also to devicetree@ as well as dt maintainers?

I didn't mean implementation. I meant that you still do not allow more
clocks! You can put into names whatever you wish but clocks are taken
from "clocks" property, not from clock-names.


Best regards,
Krzysztof