Date:   Fri, 31 May 2019 23:57:34 +0200
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Dan Murphy <dmurphy@...com>, Lee Jones <lee.jones@...aro.org>
Cc:     Pavel Machek <pavel@....cz>, broonie@...nel.org,
        lgirdwood@...il.com, linux-leds@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH v4 6/6] leds: lm36274: Introduce the TI LM36274 LED
 driver

Dan,

On 5/31/19 11:07 PM, Dan Murphy wrote:
> Hello
> 
> On 5/31/19 2:44 PM, Jacek Anaszewski wrote:
>> On 5/31/19 8:23 AM, Lee Jones wrote:
>>> On Thu, 30 May 2019, Jacek Anaszewski wrote:
>>>
>>>> On 5/30/19 9:38 AM, Lee Jones wrote:
>>>>> On Wed, 29 May 2019, Jacek Anaszewski wrote:
>>>>>
>>>>>> On 5/29/19 3:58 PM, Lee Jones wrote:
>>>>>>> On Fri, 24 May 2019, Jacek Anaszewski wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 5/23/19 9:09 PM, Dan Murphy wrote:
>>>>>>>>> Pavel
>>>>>>>>>
>>>>>>>>> Thanks for the review
>>>>>>>>>
>>>>>>>>> On 5/23/19 7:50 AM, Pavel Machek wrote:
>>>>>>>>>> Hi!
>>>>>>>>>>
>>>>>>>>>>> +++ b/drivers/leds/leds-lm36274.c
>>>>>>>>>>
>>>>>>>>>>> +static int lm36274_parse_dt(struct lm36274 *lm36274_data)
>>>>>>>>>>> +{
>>>>>>>>>>> +    struct fwnode_handle *child = NULL;
>>>>>>>>>>> +    char label[LED_MAX_NAME_SIZE];
>>>>>>>>>>> +    struct device *dev = &lm36274_data->pdev->dev;
>>>>>>>>>>> +    const char *name;
>>>>>>>>>>> +    int child_cnt;
>>>>>>>>>>> +    int ret = -EINVAL;
>>>>>>>>>>> +
>>>>>>>>>>> +    /* There should only be 1 node */
>>>>>>>>>>> +    child_cnt = device_get_child_node_count(dev);
>>>>>>>>>>> +    if (child_cnt != 1)
>>>>>>>>>>> +        return ret;
>>>>>>>>>>
>>>>>>>>>> I'd do explicit "return -EINVAL" here.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ACK
>>>>>>>>>
>>>>>>>>>>> +static int lm36274_probe(struct platform_device *pdev)
>>>>>>>>>>> +{
>>>>>>>>>>> +    struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
>>>>>>>>>>> +    struct lm36274 *lm36274_data;
>>>>>>>>>>> +    int ret;
>>>>>>>>>>> +
>>>>>>>>>>> +    lm36274_data = devm_kzalloc(&pdev->dev, 
>>>>>>>>>>> sizeof(*lm36274_data),
>>>>>>>>>>> +                    GFP_KERNEL);
>>>>>>>>>>> +    if (!lm36274_data) {
>>>>>>>>>>> +        ret = -ENOMEM;
>>>>>>>>>>> +        return ret;
>>>>>>>>>>> +    }
>>>>>>>>>>
>>>>>>>>>> And certainly do "return -ENOMEM" explicitly here.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ACK
>>>>>>>>>
>>>>>>>>>> Acked-by: Pavel Machek <pavel@....cz>
>>>>>>>>
>>>>>>>> I've done all amendments requested by Pavel and updated branch
>>>>>>>> ib-leds-mfd-regulator on linux-leds.git, but in the same time
>>>>>>>
>>>>>>> What do you mean by updated?  You cannot update an 'ib' (immutable
>>>>>>> branch).  Immutable means that it cannot change, by definition.
>>>>>>
>>>>>> We have already talked about that. Nobody has pulled so the branch
>>>>>> could have been safely updated.
>>>>>
>>>>> You have no sure way to know that.  And since I have no way to know,
>>>>> or faith that you won't update it again, pulling it now/at all would
>>>>> seem like a foolish thing to do.
>>>>
>>>> Sorry, but you are simply unjust. You're pretending to portray the
>>>> situation as if I have been notoriously causing merge conflicts in
>>>> linux-next which did not take place.
>>>>
>>>> Just to recap what this discussion is about:
>>>>
>>>> On 7 Apr 2019:
>>>>
>>>> 1. I sent pull request [0].
>>>> 2. 45 minutes later I updated it after discovering one omission [1].
>>>>     It was rather small chance for it to be pulled as quickly as that.
>>>>     And even if it happened it wouldn't have been much harmful - we
>>>>     wouldn't have lost e.g. weeks of testing in linux-next due to that
>>>>     fact.
>>>>
>>>> On 21 May 2019:
>>>>
>>>> 3. I sent another pull request [2] to you and REGULATOR maintainers.
>>>>     After it turned out that lack of feedback from REGULATOR 
>>>> maintainers
>>>>     was caused by failing to send them the exact copies of patches to
>>>>     review, I informed you about possible need for updating the branch.
>>>>     Afterwards I received a reply from you saying that you hadn't 
>>>> pulled
>>>>     the branch anyway. At that point I was sure that neither MFD nor
>>>>     REGULATOR tree contains the patches. And only after that I updated
>>>>     the branch.
>>>
>>> Here are 2 examples where you have changed immutable branches, which
>>> is 100% of the Pull Requests I have received from you.  Using that
>>> record as a benchmark, the situation hardly seems unjust.
>>>
>>>>> Until you can provide me with an assurance that you will not keep
>>>>> updating/changing the supposedly immutable pull-requests you send out,
>>>>> I won't be pulling any more in.
>>>>
>>>> I can just uphold the assurance which is implicitly assumed for anyone
>>>> who has never broken acclaimed rules. As justified above.
>>>
>>> You have broken the rules every (100% of the) time.
>>
>> Yes, I admit, I would lose in court.
>>
>>>> [0] https://lore.kernel.org/patchwork/patch/1059075/
>>>> [1] https://lore.kernel.org/patchwork/patch/1059080/
>>>> [2] https://lore.kernel.org/patchwork/patch/1077066/
>>>
>>> So we have 2 choices moving forward; you can either provide me with
>>> assurance that you have learned from this experience and will never
>>> change an *immutable* branch again, or I can continue to handle them,
>>> which has been the preference for some years.
>>>
>>> If you choose the former and adaptions need to be made in the future,
>>> the correct thing to do is create a *new*, different pull-request
>>> which has its own *new*, different tag, but uses the original tag as a
>>> base.
>>
>> I choose the former. That being said:
>>
>> Hereby I solemnly declare never ever change an immutable branch again.
>>
> So how do I proceed with the requested change by Mark B on the LM36274 
> driver.
> 
> Do I add a patch on top?
> 
> Or do I submit a patch to the regulator tree once the PR is pulled?

Won't the change be a dependency for [PATCH v4 1/6] ?

In each case, having all the commits in one set (and branch) should
simplify the integration.

-- 
Best regards,
Jacek Anaszewski

Powered by blists - more mailing lists