2018-05-30 05:09:23

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: manual merge of the regulator tree with the arm-soc tree

Hi all,

Today's linux-next merge of the regulator tree got a conflict in:

arch/arm/mach-omap1/board-ams-delta.c

between commit:

0486738928bf ("ARM: OMAP1: ams-delta: add GPIO lookup tables")

from the arm-soc tree and commit:

6059577cb28d ("regulator: fixed: Convert to use GPIO descriptor only")

from the regulator tree.

I fixed it up (see below - it may be better done) and can carry the fix
as necessary. This is now fixed as far as linux-next is concerned, but
any non trivial conflicts should be mentioned to your upstream
maintainer when your tree is submitted for merging. You may also want
to consider cooperating with the maintainer of the conflicting tree to
minimise any particularly complex conflicts.

--
Cheers,
Stephen Rothwell

diff --cc arch/arm/mach-omap1/board-ams-delta.c
index 80f54cb54276,759fa18f6ab4..000000000000
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@@ -203,10 -203,8 +203,10 @@@ static struct resource latch2_resources
},
};

- #define LATCH2_LABEL "latch2"
++#define LATCH2_LABEL "ams-delta-latch2"
+
static struct bgpio_pdata latch2_pdata = {
- .label = "ams-delta-latch2",
+ .label = LATCH2_LABEL,
.base = AMS_DELTA_LATCH2_GPIO_BASE,
.ngpio = AMS_DELTA_LATCH2_NGPIO,
};
@@@ -303,6 -288,16 +302,15 @@@ static struct platform_device modem_nre
},
};

+ static struct gpiod_lookup_table modem_nreset_gpiod_table = {
+ .dev_id = "reg-fixed-voltage",
+ .table = {
- /* The AMS_DELTA_GPIO_PIN_MODEM_NRESET is at offset 12 */
- GPIO_LOOKUP("ams-delta-latch2", 12,
++ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_MODEM_NRESET,
+ "enable", GPIO_ACTIVE_HIGH),
+ { },
+ },
+ };
+
struct modem_private_data {
struct regulator *regulator;
};
@@@ -658,15 -581,7 +666,15 @@@ static int __init late_init(void

platform_add_devices(late_devices, ARRAY_SIZE(late_devices));

+ /*
+ * As soon as devices have been registered, assign their dev_names
+ * to respective GPIO lookup tables before they are added.
+ */
+ ams_delta_lcd_gpio_table.dev_id = dev_name(&ams_delta_lcd_device.dev);
+ ams_delta_nand_gpio_table.dev_id = dev_name(&ams_delta_nand_device.dev);
+
+ gpiod_add_lookup_tables(late_gpio_tables, ARRAY_SIZE(late_gpio_tables));
-
+ gpiod_add_lookup_table(&modem_nreset_gpiod_table);
err = platform_device_register(&modem_nreset_device);
if (err) {
pr_err("Couldn't register the modem regulator device\n");


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

2018-05-30 07:30:36

by Linus Walleij

[permalink] [raw]
Subject: Re: linux-next: manual merge of the regulator tree with the arm-soc tree

On Wed, May 30, 2018 at 7:07 AM, Stephen Rothwell <[email protected]> wrote:

> Hi all,
>
> Today's linux-next merge of the regulator tree got a conflict in:
>
> arch/arm/mach-omap1/board-ams-delta.c
>
> between commit:
>
> 0486738928bf ("ARM: OMAP1: ams-delta: add GPIO lookup tables")
>
> from the arm-soc tree and commit:
>
> 6059577cb28d ("regulator: fixed: Convert to use GPIO descriptor only")
>
> from the regulator tree.
>
> I fixed it up (see below - it may be better done) and can carry the fix
> as necessary.


OMG that patch on a patch makes my head spin.

I think I just have to look at the eventual result in linux-next and see if
it makes proper sense, and rely on Janusz to test the result and help
to fix it up.

I was unaware of this concurrent gpiod conversion inside OMAP1 but
I'm happy to see that it happens. We might have some fallout, but I'm
sure Janusz is on top of things.

Yours,
Linus Walleij

2018-05-30 20:54:33

by Janusz Krzysztofik

[permalink] [raw]
Subject: Re: linux-next: manual merge of the regulator tree with the arm-soc tree

On Wednesday, May 30, 2018 7:07:11 AM CEST Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the regulator tree got a conflict in:
>
> arch/arm/mach-omap1/board-ams-delta.c
>
> between commit:
>
> 0486738928bf ("ARM: OMAP1: ams-delta: add GPIO lookup tables")
>
> from the arm-soc tree and commit:
>
> 6059577cb28d ("regulator: fixed: Convert to use GPIO descriptor only")
>
> from the regulator tree.
>
> I fixed it up (see below - it may be better done) and can carry the fix
> as necessary.

Hi Stephen,

Your fix looks pretty good to me, but anyway, I will test it and confirm in a
day or two, as soon as I have access to the device.

Thanks,
Janusz




2018-06-01 15:38:14

by Janusz Krzysztofik

[permalink] [raw]
Subject: Re: linux-next: manual merge of the regulator tree with the arm-soc tree

On Wednesday, May 30, 2018 9:29:24 AM CEST Linus Walleij wrote:
> On Wed, May 30, 2018 at 7:07 AM, Stephen Rothwell <[email protected]>
wrote:
> > Hi all,
> >
> > Today's linux-next merge of the regulator tree got a conflict in:
> > arch/arm/mach-omap1/board-ams-delta.c
> >
> > between commit:
> > 0486738928bf ("ARM: OMAP1: ams-delta: add GPIO lookup tables")
> >
> > from the arm-soc tree and commit:
> > 6059577cb28d ("regulator: fixed: Convert to use GPIO descriptor only")
> >
> > from the regulator tree.
> >
> > I fixed it up (see below - it may be better done) and can carry the fix
> > as necessary.
>
> OMG that patch on a patch makes my head spin.
>
> I think I just have to look at the eventual result in linux-next and see if
> it makes proper sense, and rely on Janusz to test the result and help
> to fix it up.

Hi,

I confirm the fix by Stephen works for me, however, the conflicting patch by
Linus breaks things a bit.

Lookup tables added to board files use function name "enable" while the
regulator uses NULL. As a result, GPIO descriptor is not matched and not
assigned to the regulator which ends up running with no control over GPIO pin.

Either the regulator driver should use the function name "enable" or that name
should be removed from lookup tables.

Thanks,
Janusz




2018-06-04 10:48:17

by Mark Brown

[permalink] [raw]
Subject: Re: linux-next: manual merge of the regulator tree with the arm-soc tree

On Fri, Jun 01, 2018 at 12:49:53AM +0200, Janusz Krzysztofik wrote:

> I confirm the fix by Stephen works for me, however, the conflicting patch by
> Linus breaks things a bit.

> Lookup tables added to board files use function name "enable" while the
> regulator uses NULL. As a result, GPIO descriptor is not matched and not
> assigned to the regulator which ends up running with no control over GPIO pin.

> Either the regulator driver should use the function name "enable" or that name
> should be removed from lookup tables.

I'll revert this one as well :(


Attachments:
(No filename) (580.00 B)
signature.asc (499.00 B)
Download all attachments

2018-06-11 11:50:32

by Linus Walleij

[permalink] [raw]
Subject: Re: linux-next: manual merge of the regulator tree with the arm-soc tree

On Mon, Jun 4, 2018 at 12:46 PM, Mark Brown <[email protected]> wrote:
> On Fri, Jun 01, 2018 at 12:49:53AM +0200, Janusz Krzysztofik wrote:
>
>> I confirm the fix by Stephen works for me, however, the conflicting patch by
>> Linus breaks things a bit.
>
>> Lookup tables added to board files use function name "enable" while the
>> regulator uses NULL. As a result, GPIO descriptor is not matched and not
>> assigned to the regulator which ends up running with no control over GPIO pin.
>
>> Either the regulator driver should use the function name "enable" or that name
>> should be removed from lookup tables.
>
> I'll revert this one as well :(

I see this is a generic problem, no idea why I didn't pass these
unnamed as NULL in the first place, probably my ignorance as usual.

I fixed it up in my patch making them all anonymous and rebasing
the rest as well. Let's see how it looks after the merge window.

I will also need to rebase on top of Janusz changes and then it
will look even better.

Janusz: would be super if you could test my patches after that!

Yours,
Linus Walleij