Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
From: Amit Kucheria <amit.kucheria@linaro.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>,
	Linux PM list <linux-pm@vger.kernel.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 17:40:28 +0530
Message-ID: <CAP245DV8jT5vj7v6vybw3Eec7wGMXRwFm=Xum5i_n4sMCHHAfg@mail.gmail.com> (raw)
In-Reply-To: <c664d247-7f9b-603f-c318-48e534aedfc9@linaro.org>

On Wed, Jul 1, 2020 at 3:15 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> 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:
> >>>>

> >>>>  /* 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.

Considering that there are no users of v1 currently, it feels a bit
over engineered, IMHO.

Also, the new UAPI doesn't need to begin at v1. Just having it start
at v2 will avoid this confusion, no?

       reply index

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200625144509.17918-1-daniel.lezcano@linaro.org>
     [not found] ` <20200625144509.17918-3-daniel.lezcano@linaro.org>
     [not found]   ` <CAP245DUMjTQr2vKirZ+FxEYWC=VQ_k+OegxQgXcKDU8ThWuCsQ@mail.gmail.com>
     [not found]     ` <0fe6837f-9b44-4578-23f2-3e4932d01122@linaro.org>
     [not found]       ` <CAP245DUG-OsSD-_CucMMQ26HpzjJhn0emfq_go923NsDq6RqOg@mail.gmail.com>
     [not found]         ` <c664d247-7f9b-603f-c318-48e534aedfc9@linaro.org>
2020-07-01 12:10           ` Amit Kucheria [this message]
2020-07-01 12:13             ` Daniel Lezcano

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='CAP245DV8jT5vj7v6vybw3Eec7wGMXRwFm=Xum5i_n4sMCHHAfg@mail.gmail.com' \
    --to=amit.kucheria@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@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

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

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

Example config snippet for mirrors

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


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