LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Amit Kucheria <amit.kucheria@linaro.org>
Cc: Zhang Rui <rui.zhang@intel.com>,
	Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
	Ram Chandrasekar <rkumbako@codeaurora.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 3/5] thermal: core: Remove old uapi generic netlink
Date: Wed, 1 Jul 2020 11:45:42 +0200
Message-ID: <c664d247-7f9b-603f-c318-48e534aedfc9@linaro.org> (raw)
In-Reply-To: <CAP245DUG-OsSD-_CucMMQ26HpzjJhn0emfq_go923NsDq6RqOg@mail.gmail.com>

On 01/07/2020 11:33, Amit Kucheria wrote:
> On Wed, Jul 1, 2020 at 2:56 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>>
>> On 30/06/2020 13:47, Amit Kucheria wrote:
>>> On Thu, Jun 25, 2020 at 8:15 PM Daniel Lezcano
>>> <daniel.lezcano@linaro.org> wrote:
>>>>
>>>> In order to set the scene for the new generic netlink thermal
>>>> management and notifications, let remove the old dead code remaining
>>>
>>> s/management/management api/
>>>
>>> s/let/let's/
>>>
>>>> in the uapi headers.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> ---
>>>>  include/linux/thermal.h      |  5 -----
>>>>  include/uapi/linux/thermal.h | 12 +-----------
>>>>  2 files changed, 1 insertion(+), 16 deletions(-)
>>>>
>>>> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
>>>> index faf7ad031e42..fc93a6348082 100644
>>>> --- a/include/linux/thermal.h
>>>> +++ b/include/linux/thermal.h
>>>> @@ -302,11 +302,6 @@ struct thermal_zone_params {
>>>>         int offset;
>>>>  };
>>>>
>>>> -struct thermal_genl_event {
>>>> -       u32 orig;
>>>> -       enum events event;
>>>> -};
>>>> -
>>>>  /**
>>>>   * struct thermal_zone_of_device_ops - scallbacks for handling DT based zones
>>>>   *
>>>> diff --git a/include/uapi/linux/thermal.h b/include/uapi/linux/thermal.h
>>>> index 96218378dda8..22df67ed9e9c 100644
>>>> --- a/include/uapi/linux/thermal.h
>>>> +++ b/include/uapi/linux/thermal.h
>>>> @@ -6,21 +6,12 @@
>>>>
>>>>  /* Adding event notification support elements */
>>>>  #define THERMAL_GENL_FAMILY_NAME                "thermal_event"
>>>> -#define THERMAL_GENL_VERSION                    0x01
>>>> +#define THERMAL_GENL_VERSION                    0x02
>>>
>>> This hunk should be removed since you set version back to 1 in the
>>> next patch and we don't actually intend to bump the version yet.
>>
>> Well, I've been very strict here for git-bisecting.
>>
>> I move to V2 because of the removal, but when adding the new genetlink
>> code, the family name changed, so we returned back to the V1 as it is a
>> new genetlink thermal brand.
> 
> I don't understand the move to v2 for an empty skeleton UAPI. For the
> purposes of bisection, couldn't you just remove all the v1 UAPI (w/o
> bumping to v2) and then add a new UAPI in the next patch?
> 
>> The name is change because it is no longer event based but also sampling
>> and commands.
> 
> In this case, just to avoid any confusion, the new UAPI could be v2
> making the transition clear in case of bisection.
> 
> I'm afraid the v1->v2->v1 is a bit more confusing.

Let me elaborate a bit:

Why there is this patch ?
- By removing this code first, the next patch will just contain
additions, I thought it would be clearer

Why increase the version here ?
- Code must continue to compile and as the 'thermal_event' family is now
different from V1, the version is changed

Why the version goes to V1 in the next patch ?
- The family name is changed as it is not doing event only, so it is a
new netlink thermal protocol and we begin at V1

So the main reason of this patch is to be very strict in the iteration
changes. May be it is too much, in this case I can merge this patch with
4/5, the old netlink protocol removal will be lost in the addition of
the new protocol. I'm fine with that if you think it is simpler.




-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-25 14:45 [PATCH v2 1/5] thermal: core: Add helpers to browse the cdev, tz and governor list Daniel Lezcano
2020-06-25 14:45 ` [PATCH v2 2/5] thermal: core: Get thermal zone by id Daniel Lezcano
2020-06-30 11:54   ` Amit Kucheria
2020-06-30 15:16   ` Zhang Rui
2020-06-25 14:45 ` [PATCH v2 3/5] thermal: core: Remove old uapi generic netlink Daniel Lezcano
2020-06-30 11:47   ` Amit Kucheria
2020-07-01  9:26     ` Daniel Lezcano
2020-07-01  9:33       ` Amit Kucheria
2020-07-01  9:45         ` Daniel Lezcano [this message]
2020-07-01 12:10           ` Amit Kucheria
2020-07-01 12:13             ` Daniel Lezcano
2020-06-25 14:45 ` [PATCH v2 4/5] thermal: core: genetlink support for events/cmd/sampling Daniel Lezcano
2020-06-30 11:45   ` Amit Kucheria
2020-06-30 16:12   ` Zhang Rui
2020-06-30 18:32     ` Daniel Lezcano
2020-07-01  7:41       ` Zhang Rui
2020-07-01  8:22         ` Daniel Lezcano
2020-07-01 15:49         ` Srinivas Pandruvada
2020-07-01 16:31           ` Daniel Lezcano
2020-07-01 16:41             ` Srinivas Pandruvada
2020-06-25 14:45 ` [PATCH v2 5/5] thermal: core: Add notifications call in the framework Daniel Lezcano
2020-06-30 11:49   ` Amit Kucheria
2020-06-30 11:58     ` Daniel Lezcano
2020-06-30 11:46 ` [PATCH v2 1/5] thermal: core: Add helpers to browse the cdev, tz and governor list Amit Kucheria
2020-06-30 15:09   ` Zhang Rui
2020-06-30 18:46     ` Amit Kucheria
2020-07-01  7:08       ` Daniel Lezcano
2020-07-01  7:35     ` Daniel Lezcano
2020-07-01  7:57       ` Zhang Rui
2020-07-01  9:26         ` Amit Kucheria
2020-07-01  9:54           ` Daniel Lezcano
2020-07-01  9:50         ` Daniel Lezcano
2020-07-02 13:53           ` Zhang Rui

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=c664d247-7f9b-603f-c318-48e534aedfc9@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=amit.kucheria@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rkumbako@codeaurora.org \
    --cc=rui.zhang@intel.com \
    --cc=srinivas.pandruvada@linux.intel.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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lkml.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lkml.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lkml.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lkml.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lkml.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lkml.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lkml.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lkml.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lkml.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lkml.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git