| Message ID | 20190522192733.13422-1-dmurphy@ti.com |
|---|---|
| Headers | show |
| Series | LM36274 Introduction | expand |
On Wed 2019-05-22 14:27:29, Dan Murphy wrote: > Add the LM36274 backlight driver with regulator support. > This is a multi-function device for backlight applications. > > Backlight properties will be documented in it's a supplemental > bindings document. > > Regulator support is documented in the regulator/lm363x-regulator.txt > > http://www.ti.com/lit/ds/symlink/lm36274.pdf > > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Dan Murphy <dmurphy@ti.com> Acked-by: Pavel Machek <pavel@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Wed 2019-05-22 14:27:30, Dan Murphy wrote: > Add the LM36274 register support to the ti-lmu MFD driver. > > Signed-off-by: Dan Murphy <dmurphy@ti.com> Acked-by: Pavel Machek <pavel@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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. > +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. Acked-by: Pavel Machek <pavel@ucw.cz> Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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@ucw.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 dropped the merge from the for-next. We will proceed further once we clarify the issue of cross-merging recently raised again by Linus. -- Best regards, Jacek Anaszewski
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@ucw.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. > dropped the merge from the for-next. > > We will proceed further once we clarify the issue of cross-merging > recently raised again by Linus. > -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
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@ucw.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. -- Best regards, Jacek Anaszewski
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@ucw.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. > 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. [0] https://lore.kernel.org/patchwork/patch/1059075/ [1] https://lore.kernel.org/patchwork/patch/1059080/ [2] https://lore.kernel.org/patchwork/patch/1077066/ -- Best regards, Jacek Anaszewski
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@ucw.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? Dan
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@ucw.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
Jacek On 5/31/19 4:57 PM, Jacek Anaszewski wrote: > 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@ucw.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] ? > Yes thats why I am asking as we would need to change the branch. Dan > In each case, having all the commits in one set (and branch) should > simplify the integration. >
Dan, On 6/1/19 12:41 AM, Dan Murphy wrote: > Jacek > > On 5/31/19 4:57 PM, Jacek Anaszewski wrote: >> 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@ucw.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] ? >> > Yes thats why I am asking as we would need to change the branch. I will need to send another pull request anyway - I haven't created new one after updating the branch so far, so for now we are free to change it. -- Best regards, Jacek Anaszewski