Introduce the LM36274 LED driver. This driver uses the ti-lmu
MFD driver to probe this LED driver. The driver configures only the
LED registers and enables the outputs according to the config file.
The driver utilizes the TI LMU (Lighting Management Unit) LED common
framework to set the brightness bits.
Signed-off-by: Dan Murphy <[email protected]>
---
drivers/leds/Kconfig | 7 ++
drivers/leds/Makefile | 1 +
drivers/leds/leds-lm36274.c | 174 ++++++++++++++++++++++++++++++++++++
3 files changed, 182 insertions(+)
create mode 100644 drivers/leds/leds-lm36274.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 255fdd5e8491..db83a3feca01 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -791,6 +791,13 @@ config LEDS_LM3697
Say Y to enable the LM3697 LED driver for TI LMU devices.
This supports the LED device LM3697.
+config LEDS_LM36274
+ tristate "LED driver for LM36274"
+ depends on LEDS_TI_LMU_COMMON
+ help
+ Say Y to enable the LM36274 LED driver for TI LMU devices.
+ This supports the LED device LM36274.
+
comment "LED Triggers"
source "drivers/leds/trigger/Kconfig"
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index 8ab825c8b5c3..c229432b7fe7 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_LEDS_SC27XX_BLTC) += leds-sc27xx-bltc.o
obj-$(CONFIG_LEDS_LM3601X) += leds-lm3601x.o
obj-$(CONFIG_LEDS_TI_LMU_COMMON) += leds-ti-lmu-common.o
obj-$(CONFIG_LEDS_LM3697) += leds-lm3697.o
+obj-$(CONFIG_LEDS_LM36274) += leds-lm36274.o
# LED SPI Drivers
obj-$(CONFIG_LEDS_CR0014114) += leds-cr0014114.o
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
new file mode 100644
index 000000000000..b47786d36d21
--- /dev/null
+++ b/drivers/leds/leds-lm36274.c
@@ -0,0 +1,174 @@
+// SPDX-License-Identifier: GPL-2.0
+// TI LM36274 LED chip family driver
+// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/leds.h>
+#include <linux/leds-ti-lmu-common.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#include <linux/mfd/ti-lmu.h>
+#include <linux/mfd/ti-lmu-register.h>
+
+#include <uapi/linux/uleds.h>
+
+#define LM36274_MAX_STRINGS 4
+#define LM36274_BL_EN BIT(4)
+
+/**
+ * struct lm36274
+ * @pdev: platform device
+ * @led_dev: led class device
+ * @lmu_data: Register and setting values for common code
+ * @regmap: Devices register map
+ * @dev: Pointer to the devices device struct
+ * @led_sources - The LED strings supported in this array
+ * @num_leds - Number of LED strings are supported in this array
+ */
+struct lm36274 {
+ struct platform_device *pdev;
+ struct led_classdev led_dev;
+ struct ti_lmu_bank lmu_data;
+ struct regmap *regmap;
+ struct device *dev;
+
+ u32 led_sources[LM36274_MAX_STRINGS];
+ int num_leds;
+};
+
+static int lm36274_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brt_val)
+{
+ struct lm36274 *led = container_of(led_cdev, struct lm36274, led_dev);
+
+ return ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+}
+
+static int lm36274_init(struct lm36274 *lm36274_data)
+{
+ int enable_val = 0;
+ int i;
+
+ for (i = 0; i < lm36274_data->num_leds; i++)
+ enable_val |= (1 << lm36274_data->led_sources[i]);
+
+ if (!enable_val) {
+ dev_err(lm36274_data->dev, "No LEDs were enabled\n");
+ return -EINVAL;
+ }
+
+ enable_val |= LM36274_BL_EN;
+
+ return regmap_write(lm36274_data->regmap, LM36274_REG_BL_EN,
+ enable_val);
+}
+
+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;
+
+ device_for_each_child_node(dev, child) {
+ ret = fwnode_property_read_string(child, "label", &name);
+ if (ret)
+ snprintf(label, sizeof(label),
+ "%s::", lm36274_data->pdev->name);
+ else
+ snprintf(label, sizeof(label),
+ "%s:%s", lm36274_data->pdev->name, name);
+
+ lm36274_data->num_leds = fwnode_property_read_u32_array(child,
+ "led-sources",
+ NULL, 0);
+ if (lm36274_data->num_leds <= 0)
+ return -ENODEV;
+
+ ret = fwnode_property_read_u32_array(child, "led-sources",
+ lm36274_data->led_sources,
+ lm36274_data->num_leds);
+ if (ret) {
+ dev_err(dev, "led-sources property missing\n");
+ return -EINVAL;
+ }
+
+ fwnode_property_read_string(child, "linux,default-trigger",
+ &lm36274_data->led_dev.default_trigger);
+
+ }
+
+ lm36274_data->lmu_data.regmap = lm36274_data->regmap;
+ lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+ lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+ lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+
+ lm36274_data->led_dev.name = label;
+ lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+ lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;
+
+ return ret;
+}
+
+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;
+ }
+
+ lm36274_data->pdev = pdev;
+ lm36274_data->dev = lmu->dev;
+ lm36274_data->regmap = lmu->regmap;
+ dev_set_drvdata(&pdev->dev, lm36274_data);
+
+ ret = lm36274_parse_dt(lm36274_data);
+ if (ret) {
+ dev_err(lm36274_data->dev, "Failed to parse DT node\n");
+ return ret;
+ }
+
+ ret = lm36274_init(lm36274_data);
+ if (ret) {
+ dev_err(lm36274_data->dev, "Failed to init the device\n");
+ return ret;
+ }
+
+ return devm_led_classdev_register(lm36274_data->dev,
+ &lm36274_data->led_dev);
+}
+
+static const struct of_device_id of_lm36274_leds_match[] = {
+ { .compatible = "ti,lm36274-backlight", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_lm36274_leds_match);
+
+static struct platform_driver lm36274_driver = {
+ .probe = lm36274_probe,
+ .driver = {
+ .name = "lm36274-leds",
+ },
+};
+module_platform_driver(lm36274_driver)
+
+MODULE_DESCRIPTION("Texas Instruments LM36274 LED driver");
+MODULE_AUTHOR("Dan Murphy <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.21.0.5.gaeb582a983
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 <[email protected]>
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
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 <[email protected]>
> Pavel
>
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 <[email protected]>
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 <[email protected]>
>
> 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 <[email protected]>
>>
>> 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 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 <[email protected]>
> > >
> > > 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.
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.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
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 <[email protected]>
>>>>
>>>> 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
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 <[email protected]>
> > > > >
> > > > > 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.
> [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.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
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 <[email protected]>
>>>>>>
>>>>>> 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.
--
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 <[email protected]>
>>>>>>>
>>>>>>> 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 <[email protected]>
>>>>>>>>
>>>>>>>> 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 <[email protected]>
>>>>>>>>>
>>>>>>>>> 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 <[email protected]>
>>>>>>>>>>
>>>>>>>>>> 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