All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
To: Youngmin Nam <youngmin.nam@samsung.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	daniel.lezcano@linaro.org
Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, pullip.cho@samsung.com,
	hoony.yu@samsung.com, hajun.sung@samsung.com,
	myung-su.cha@samsung.com, kgene@kernel.org
Subject: Re: [PATCH v1 1/2] clocksource/drivers/exynos_mct_v2: introduce Exynos MCT version 2 driver for next Exynos SoC
Date: Tue, 26 Oct 2021 13:00:58 +0200	[thread overview]
Message-ID: <11ca846b-c8d6-37c9-8ee2-4740fa66d974@canonical.com> (raw)
In-Reply-To: <20211026104518.GA40630@perf>

On 26/10/2021 12:45, Youngmin Nam wrote:
> On Tue, Oct 26, 2021 at 09:10:28AM +0200, Krzysztof Kozlowski wrote:
>> On 26/10/2021 03:47, Youngmin Nam wrote:
>>>> If everyone added a new driver to avoid integrating with existing code,
>>>> we would have huge kernel with thousands of duplicated solutions. The
>>>> kernel also would be unmaintained.
>>>>
>>>> Such arguments were brought before several times - "I don't want to
>>>> integrating with existing code", "My use case is different", "I would
>>>> need to test the other cases", "It's complicated for me".
>>>>
>>>> Instead of pushing a new vendor driver you should integrate it with
>>>> existing code.
>>>>
>>> Let me ask you one question.
>>> If we maintain as one driver, how can people who don't have the new MCT test the new driver?
>>
>> I assume you talk about a case when someone else later changes something
>> in the driver. Such person doesn't necessarily have to test it. The same
>> as in all other cases (Exynos MCT is not special here): just ask for
>> testing on platform one doesn't have.
>>
>> Even if you submit this as separate driver, there is the exact same
>> problem. People will change the MCTv2 driver without access to hardware.
>>
> Yes, I can test the new MCT driver if someone ask for testing after modifying the new driver.
> But in this case, we don't need to test the previous MCT driver. We have only to test the new MCT driver.

Like with everything in Linux kernel. We merge instead of duplicate.
It's not an argument.

>> None of these differ for Exynos MCT from other drivers, e.g. mentioned
>> Samsung PMIC drivers, recently modified (by Will and Sam) the SoC clock
>> drivers or the ChipID drivers (changed by Chanho).
> From HW point of view, the previous MCT is almost 10-year-old IP without any major change and
> it will not be used on next new Exynos SoC.
> MCTv2 is the totally newly designed IP and it will replace the Exynos system timer.
> Device driver would be dependent with H/W. We are going to apply a lot of changes for this new MCT.
> For maintenance, I think we should separate the new MCT driver for maintenance.
> 

There are several similarities which actually suggest that you
exaggerate the differences.

The number of interrupts is the same (4+8 in older one, 12 in new one...).
You assign the MCT priority also as higher than Architected Timer
(+Cc Will and Mark - is it ok for you?)
    evt->rating = 500;      /* use value higher than ARM arch timer *

All these point that block is not different. Again, let me repeat, we
support old Samsung PMICs with new Samsung PMICs in one driver. Even
though the "old one" won't be changed, as you mentioned here. The same
Samsung SoC clock drivers are used for old Exynos and for new ones...
Similarly to pinctrl drivers. The same ChipId.

Everywhere we follow the same concept of unification instead of
duplication. Maybe Exynos MCT timer is an exception but you did not
provide any arguments supporting this. Why Exynos MCTv2 should be
treated differently than Exynos850 clocks, chipid, pinctrl and other blocks?

Daniel,
Any preferences from you? Integrating MCT into existing driver (thus
growing it) or having a new one?

Best regards,
Krzysztof

WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
To: Youngmin Nam <youngmin.nam@samsung.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	daniel.lezcano@linaro.org
Cc: tglx@linutronix.de, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, pullip.cho@samsung.com,
	hoony.yu@samsung.com, hajun.sung@samsung.com,
	myung-su.cha@samsung.com, kgene@kernel.org
Subject: Re: [PATCH v1 1/2] clocksource/drivers/exynos_mct_v2: introduce Exynos MCT version 2 driver for next Exynos SoC
Date: Tue, 26 Oct 2021 13:00:58 +0200	[thread overview]
Message-ID: <11ca846b-c8d6-37c9-8ee2-4740fa66d974@canonical.com> (raw)
In-Reply-To: <20211026104518.GA40630@perf>

On 26/10/2021 12:45, Youngmin Nam wrote:
> On Tue, Oct 26, 2021 at 09:10:28AM +0200, Krzysztof Kozlowski wrote:
>> On 26/10/2021 03:47, Youngmin Nam wrote:
>>>> If everyone added a new driver to avoid integrating with existing code,
>>>> we would have huge kernel with thousands of duplicated solutions. The
>>>> kernel also would be unmaintained.
>>>>
>>>> Such arguments were brought before several times - "I don't want to
>>>> integrating with existing code", "My use case is different", "I would
>>>> need to test the other cases", "It's complicated for me".
>>>>
>>>> Instead of pushing a new vendor driver you should integrate it with
>>>> existing code.
>>>>
>>> Let me ask you one question.
>>> If we maintain as one driver, how can people who don't have the new MCT test the new driver?
>>
>> I assume you talk about a case when someone else later changes something
>> in the driver. Such person doesn't necessarily have to test it. The same
>> as in all other cases (Exynos MCT is not special here): just ask for
>> testing on platform one doesn't have.
>>
>> Even if you submit this as separate driver, there is the exact same
>> problem. People will change the MCTv2 driver without access to hardware.
>>
> Yes, I can test the new MCT driver if someone ask for testing after modifying the new driver.
> But in this case, we don't need to test the previous MCT driver. We have only to test the new MCT driver.

Like with everything in Linux kernel. We merge instead of duplicate.
It's not an argument.

>> None of these differ for Exynos MCT from other drivers, e.g. mentioned
>> Samsung PMIC drivers, recently modified (by Will and Sam) the SoC clock
>> drivers or the ChipID drivers (changed by Chanho).
> From HW point of view, the previous MCT is almost 10-year-old IP without any major change and
> it will not be used on next new Exynos SoC.
> MCTv2 is the totally newly designed IP and it will replace the Exynos system timer.
> Device driver would be dependent with H/W. We are going to apply a lot of changes for this new MCT.
> For maintenance, I think we should separate the new MCT driver for maintenance.
> 

There are several similarities which actually suggest that you
exaggerate the differences.

The number of interrupts is the same (4+8 in older one, 12 in new one...).
You assign the MCT priority also as higher than Architected Timer
(+Cc Will and Mark - is it ok for you?)
    evt->rating = 500;      /* use value higher than ARM arch timer *

All these point that block is not different. Again, let me repeat, we
support old Samsung PMICs with new Samsung PMICs in one driver. Even
though the "old one" won't be changed, as you mentioned here. The same
Samsung SoC clock drivers are used for old Exynos and for new ones...
Similarly to pinctrl drivers. The same ChipId.

Everywhere we follow the same concept of unification instead of
duplication. Maybe Exynos MCT timer is an exception but you did not
provide any arguments supporting this. Why Exynos MCTv2 should be
treated differently than Exynos850 clocks, chipid, pinctrl and other blocks?

Daniel,
Any preferences from you? Integrating MCT into existing driver (thus
growing it) or having a new one?

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2021-10-26 11:01 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20211021055104epcas2p4bd5278e58860af8c136a850c0f080087@epcas2p4.samsung.com>
2021-10-21  6:18 ` [PATCH v1 0/2] Indroduce Exynos Multi Core Timer version 2 Youngmin Nam
2021-10-21  6:18   ` Youngmin Nam
     [not found]   ` <CGME20211021055112epcas2p278145beb21cd6cc4217813a41c1e1407@epcas2p2.samsung.com>
2021-10-21  6:18     ` [PATCH v1 1/2] clocksource/drivers/exynos_mct_v2: introduce Exynos MCT version 2 driver for next Exynos SoC Youngmin Nam
2021-10-21  6:18       ` Youngmin Nam
2021-10-21  6:18       ` Krzysztof Kozlowski
2021-10-21  6:18         ` Krzysztof Kozlowski
2021-10-21  8:26         ` Youngmin Nam
2021-10-21  8:26           ` Youngmin Nam
2021-10-21  8:07           ` Krzysztof Kozlowski
2021-10-21  8:07             ` Krzysztof Kozlowski
2021-10-22  4:21             ` Youngmin Nam
2021-10-22  4:21               ` Youngmin Nam
2021-10-25  8:18               ` Krzysztof Kozlowski
2021-10-25  8:18                 ` Krzysztof Kozlowski
2021-10-26  1:47                 ` Youngmin Nam
2021-10-26  1:47                   ` Youngmin Nam
2021-10-26  7:10                   ` Krzysztof Kozlowski
2021-10-26  7:10                     ` Krzysztof Kozlowski
2021-10-26 10:45                     ` Youngmin Nam
2021-10-26 10:45                       ` Youngmin Nam
2021-10-26 11:00                       ` Krzysztof Kozlowski
2021-10-26 11:00                         ` Krzysztof Kozlowski
     [not found]                         ` <CGME20211027011125epcas2p2916524051416ede854b750c91a19073b@epcas2p2.samsung.com>
2021-10-27  1:38                           ` Youngmin Nam
2021-10-27  1:38                             ` Youngmin Nam
2021-10-27  6:39                             ` Krzysztof Kozlowski
2021-10-27  6:39                               ` Krzysztof Kozlowski
2021-11-01  6:04                               ` Youngmin Nam
2021-11-01  6:04                                 ` Youngmin Nam
2021-10-27  7:34                             ` Will Deacon
2021-10-27  7:34                               ` Will Deacon
2021-10-29  3:54                               ` Youngmin Nam
2021-10-29  3:54                                 ` Youngmin Nam
2021-10-26 11:00                       ` Krzysztof Kozlowski [this message]
2021-10-26 11:00                         ` Krzysztof Kozlowski
2021-10-27  8:38       ` Krzysztof Kozlowski
2021-10-27  8:38         ` Krzysztof Kozlowski
2021-11-01  5:25         ` Youngmin Nam
2021-11-01  5:25           ` Youngmin Nam
     [not found]   ` <CGME20211021055115epcas2p158fbf3ac61d3deeb5995bd100d7edef1@epcas2p1.samsung.com>
2021-10-21  6:18     ` [PATCH v1 2/2] dt-bindings: timer: samsung,s5e99xx-mct: Document s5e99xx-mct bindings Youngmin Nam
2021-10-21  6:18       ` Youngmin Nam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11ca846b-c8d6-37c9-8ee2-4740fa66d974@canonical.com \
    --to=krzysztof.kozlowski@canonical.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=hajun.sung@samsung.com \
    --cc=hoony.yu@samsung.com \
    --cc=kgene@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=myung-su.cha@samsung.com \
    --cc=pullip.cho@samsung.com \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    --cc=youngmin.nam@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.