[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Paul Kocialkowski]

This switches a few interrupt definitions that were using
GPIO_ACTIVE_HIGH as IRQ type, which is invalid.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
index 271505e..30a77ec 100644
--- a/arch/arm/boot/dts/tegra124-nyan.dtsi
+++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
@@ -59,7 +59,7 @@
 			compatible = "maxim,max98090";
 			reg = <0x10>;
 			interrupt-parent = <&gpio>;
-			interrupts = <TEGRA_GPIO(H, 4) GPIO_ACTIVE_HIGH>;
+			interrupts = <TEGRA_GPIO(H, 4) IRQ_TYPE_EDGE_FALLING>;
 		};
 
 		temperature-sensor at 4c {
@@ -325,7 +325,7 @@
 					reg = <0x9>;
 					interrupt-parent = <&gpio>;
 					interrupts = <TEGRA_GPIO(J, 0)
-							GPIO_ACTIVE_HIGH>;
+							IRQ_TYPE_EDGE_BOTH>;
 					ti,ac-detect-gpios = <&gpio
 							TEGRA_GPIO(J, 0)
 							GPIO_ACTIVE_HIGH>;
-- 
2.9.3

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Paul Kocialkowski]

Nyan boards come with an embedded controller that controls when to
enable and disable the charge. Thus, it should not be left up to the
kernel to handle that.

Using the ti,external-control property allows specifying this use-case.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/tegra124-nyan.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
index 30a77ec..18db797 100644
--- a/arch/arm/boot/dts/tegra124-nyan.dtsi
+++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
@@ -329,6 +329,7 @@
 					ti,ac-detect-gpios = <&gpio
 							TEGRA_GPIO(J, 0)
 							GPIO_ACTIVE_HIGH>;
+					ti,external-control;
 				};
 
 				battery: sbs-battery at b {
-- 
2.9.3

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Jon Hunter]

On 28/08/16 18:32, Paul Kocialkowski wrote:
> Nyan boards come with an embedded controller that controls when to
> enable and disable the charge. Thus, it should not be left up to the
> kernel to handle that.
> 
> Using the ti,external-control property allows specifying this use-case.

So the bq24735 is populated under the EC's 'i2c-tunnel' property which
is there to specifically interface it's child devices to the host. So I
am a bit confused why this is expose to the host if it should not be used?

Again you may right and I did find the original series [0] for this
which specifically references the Acer Chromebook that needs this.
However, I am not sure why this was never populated? Is there any other
history here?

What is the actual problem you see without making this change? The
original series states ...

"On Acer Chromebook 13 (CB5-311) this module fails to load if the
charger is not inserted, and will error when it is removed."

Cheers
Jon

[0] http://marc.info/?l=linux-pm&m=145447948705686&w=2

-- 
nvpublic

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Paul Kocialkowski]

Le mardi 20 septembre 2016 ? 18:40 +0100, Jon Hunter a ?crit :
> On 28/08/16 18:32, Paul Kocialkowski wrote:
> >?
> > Nyan boards come with an embedded controller that controls when to
> > enable and disable the charge. Thus, it should not be left up to the
> > kernel to handle that.
> >?
> > Using the ti,external-control property allows specifying this use-case.
>?
> So the bq24735 is populated under the EC's 'i2c-tunnel' property which
> is there to specifically interface it's child devices to the host. So I
> am a bit confused why this is expose to the host if it should not be used?

Well, it needs to access the information in the read-only registers provided by
the chip, which is allowed by the setup in place that you described.

However, the EC has its internal state machine that decides when to start
charging, etc and so should be the only one to write registers, to avoid
conflicts.

> Again you may right and I did find the original series [0] for this
> which specifically references the Acer Chromebook that needs this.
> However, I am not sure why this was never populated? Is there any other
> history here?

I am also confused about why it wasn't applied earlier. However, the cros kernel
is using the very same scheme.

> What is the actual problem you see without making this change?

There is a risk of conflict (even though it's probably not that significant),
given the low variety of possible cases here. The idea is simply to say that the
EC is in charge and to let it do its job without interfering.

> The original series states ...
>?
> "On Acer Chromebook 13 (CB5-311) this module fails to load if the
> charger is not inserted, and will error when it is removed."

I'm confused about that comment. At this point (and with this patch), it works
normally.

> Cheers
> Jon
>?
> [0] http://marc.info/?l=linux-pm&m=145447948705686&w=2

Cheers,

--?
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/19d6ecea/attachment.sig>

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Jon Hunter]

On 20/09/16 19:02, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mardi 20 septembre 2016 ? 18:40 +0100, Jon Hunter a ?crit :
>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>  
>>> Nyan boards come with an embedded controller that controls when to
>>> enable and disable the charge. Thus, it should not be left up to the
>>> kernel to handle that.
>>>  
>>> Using the ti,external-control property allows specifying this use-case.
>>  
>> So the bq24735 is populated under the EC's 'i2c-tunnel' property which
>> is there to specifically interface it's child devices to the host. So I
>> am a bit confused why this is expose to the host if it should not be used?
> 
> Well, it needs to access the information in the read-only registers provided by
> the chip, which is allowed by the setup in place that you described.

Is this to expose the current state to the kernel so we can monitor the
battery state?

> However, the EC has its internal state machine that decides when to start
> charging, etc and so should be the only one to write registers, to avoid
> conflicts.
> 
>> Again you may right and I did find the original series [0] for this
>> which specifically references the Acer Chromebook that needs this.
>> However, I am not sure why this was never populated? Is there any other
>> history here?
> 
> I am also confused about why it wasn't applied earlier. However, the cros kernel
> is using the very same scheme.

Do you have a reference?

>> What is the actual problem you see without making this change?
> 
> There is a risk of conflict (even though it's probably not that significant),
> given the low variety of possible cases here. The idea is simply to say that the
> EC is in charge and to let it do its job without interfering.
> 
>> The original series states ...
>>  
>> "On Acer Chromebook 13 (CB5-311) this module fails to load if the
>> charger is not inserted, and will error when it is removed."
> 
> I'm confused about that comment. At this point (and with this patch), it works
> normally.

Ok, I think Thierry prefers to only apply fixes for problems that can be
reproduced. Is there a simple way to check the battery status and
charging status via say the sysfs? If I can test that this has no
negative impact may be it is ok.

Cheers
Jon

-- 
nvpublic

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Paul Kocialkowski]

Le mercredi 21 septembre 2016 ? 08:30 +0100, Jon Hunter a ?crit?:
> On 20/09/16 19:02, Paul Kocialkowski wrote:
> > 
> > * PGP Signed by an unknown key
> > 
> > Le mardi 20 septembre 2016 ? 18:40 +0100, Jon Hunter a ?crit :
> > > 
> > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > 
> > > > ?
> > > > Nyan boards come with an embedded controller that controls when to
> > > > enable and disable the charge. Thus, it should not be left up to the
> > > > kernel to handle that.
> > > > ?
> > > > Using the ti,external-control property allows specifying this use-case.
> > > ?
> > > So the bq24735 is populated under the EC's 'i2c-tunnel' property which
> > > is there to specifically interface it's child devices to the host. So I
> > > am a bit confused why this is expose to the host if it should not be used?
> > 
> > Well, it needs to access the information in the read-only registers provided
> > by
> > the chip, which is allowed by the setup in place that you described.
> 
> Is this to expose the current state to the kernel so we can monitor the
> battery state?

Yes, that is correct.

> > However, the EC has its internal state machine that decides when to start
> > charging, etc and so should be the only one to write registers, to avoid
> > conflicts.
> > 
> > > 
> > > Again you may right and I did find the original series [0] for this
> > > which specifically references the Acer Chromebook that needs this.
> > > However, I am not sure why this was never populated? Is there any other
> > > history here?
> > 
> > I am also confused about why it wasn't applied earlier. However, the cros
> > kernel
> > is using the very same scheme.
> 
> Do you have a reference?

Sure thing, there's a similar commit for the dts[0] and one for the driver[1]
(which was already merged in mainline).

> > > What is the actual problem you see without making this change?
> > 
> > There is a risk of conflict (even though it's probably not that
> > significant),
> > given the low variety of possible cases here. The idea is simply to say that
> > the
> > EC is in charge and to let it do its job without interfering.
> > 
> > > 
> > > The original series states ...
> > > ?
> > > "On Acer Chromebook 13 (CB5-311) this module fails to load if the
> > > charger is not inserted, and will error when it is removed."
> > 
> > I'm confused about that comment. At this point (and with this patch), it
> > works
> > normally.
> 
> Ok, I think Thierry prefers to only apply fixes for problems that can be
> reproduced. Is there a simple way to check the battery status and
> charging status via say the sysfs? If I can test that this has no
> negative impact may be it is ok.

Sure, this is exported at: /sys/class/power_supply/bq24735 at 5-0009
Also, note that the power-supply next branch[2] has some more fixes for the
bq24735 driver.

Cheers,

[0]:?https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e25a91f87af41e29012a4e2dd7a9ab725efd308e
[1]: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6b34e53d506b44f911d0fd246ccdc8b4e942e4ae
[2]: https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/log/?h=for-next

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/be9788f1/attachment.sig>

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Jon Hunter]

On 21/09/16 08:56, Paul Kocialkowski wrote:

...

> Sure, this is exported at: /sys/class/power_supply/bq24735 at 5-0009
> Also, note that the power-supply next branch[2] has some more fixes for the
> bq24735 driver.

I tested the bq24735 on next-20160919 without any of your changes and 
when connecting or disconnecting the charger I see the following panic.
Do you see this?

Jon

/ # [   30.120384] Unable to handle kernel NULL pointer dereference at virtual address 00000004
[   30.128489] pgd = c0004000
[   30.131187] [00000004] *pgd=00000000
[   30.134759] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
[   30.140141] Modules linked in:
[   30.143192] CPU: 1 PID: 71 Comm: kworker/1:1 Not tainted 4.8.0-rc6-next-20160919-00002-gbc9771827865-dirty #574
[   30.153254] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[   30.159509] Workqueue: events power_supply_changed_work
[   30.164723] task: ee19f880 task.stack: ee1a0000
[   30.169239] PC is at sbs_external_power_changed+0x50/0x5c
[   30.174624] LR is at mod_timer+0x194/0x270
[   30.178705] pc : [<c05b0c58>]    lr : [<c017cda0>]    psr: 60000113
[   30.178705] sp : ee1a1eb8  ip : 80000000  fp : ee8e3680
[   30.190155] r10: eef98580  r9 : 00000000  r8 : 00000000
[   30.195363] r7 : ee303000  r6 : c05afef8  r5 : ee3e03f8  r4 : ee3e03d0
[   30.201872] r3 : 00000000  r2 : 00000000  r1 : 40000193  r0 : 00000001
[   30.208382] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   30.215497] Control: 10c5387d  Table: adcf006a  DAC: 00000051
[   30.221226] Process kworker/1:1 (pid: 71, stack limit = 0xee1a0210)
[   30.227474] Stack: (0xee1a1eb8 to 0xee1a2000)
[   30.231816] 1ea0:                                                       edc0bc00 ee303000
[   30.239973] 1ec0: c05afef8 c05aff2c edc0bc20 c045ec1c eef95c80 eef95c80 00000001 eea82d5c
[   30.248136] 1ee0: edc0bd98 00000000 ee3031c0 ee303218 c0eab740 c05afcc4 ee8e3680 ee3031c0
[   30.256296] 1f00: eef9b800 00000000 eef98580 c0135824 eef98598 c0e02100 eef98580 ee8e3698
[   30.264453] 1f20: 00000008 eef98598 c0e02100 ee1a0000 eef98580 c0135a74 ee840f80 ee8e3680
[   30.272610] 1f40: c0135a3c 00000000 ee840f80 ee8e3680 c0135a3c 00000000 00000000 00000000
[   30.280766] 1f60: 00000000 c013af18 00000000 00000000 00000000 ee8e3680 00000000 00000000
[   30.288922] 1f80: ee1a1f80 ee1a1f80 00000000 00000000 ee1a1f90 ee1a1f90 ee1a1fac ee840f80
[   30.297078] 1fa0: c013ae3c 00000000 00000000 c01078b8 00000000 00000000 00000000 00000000
[   30.305234] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   30.313389] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[   30.321555] [<c05b0c58>] (sbs_external_power_changed) from [<c05aff2c>] (__power_supply_changed_work+0x34/0x3c)
[   30.331623] [<c05aff2c>] (__power_supply_changed_work) from [<c045ec1c>] (class_for_each_device+0x4c/0xb4)
[   30.341254] [<c045ec1c>] (class_for_each_device) from [<c05afcc4>] (power_supply_changed_work+0x5c/0xb0)
[   30.350713] [<c05afcc4>] (power_supply_changed_work) from [<c0135824>] (process_one_work+0x124/0x33c)
[   30.359912] [<c0135824>] (process_one_work) from [<c0135a74>] (worker_thread+0x38/0x4d4)
[   30.367983] [<c0135a74>] (worker_thread) from [<c013af18>] (kthread+0xdc/0xf4)
[   30.375187] [<c013af18>] (kthread) from [<c01078b8>] (ret_from_fork+0x14/0x3c)
[   30.382391] Code: e5911000 e3a00004 ebee0b88 e5943008 (e5933004) 
[   30.388504] ---[ end trace 083d55597e9a2254 ]---
[   30.393140] Unable to handle kernel paging request at virtual address ffffffec
[   30.400342] pgd = c0004000
[   30.403038] [ffffffec] *pgd=afffd861, *pte=00000000, *ppte=00000000
[   30.409309] Internal error: Oops: 37 [#2] PREEMPT SMP ARM
[   30.414690] Modules linked in:
[   30.417738] CPU: 1 PID: 71 Comm: kworker/1:1 Tainted: G      D         4.8.0-rc6-next-20160919-00002-gbc9771827865-dirty #574
[   30.429013] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
[   30.435265] task: ee19f880 task.stack: ee1a0000
[   30.439782] PC is at kthread_data+0x4/0xc
[   30.443779] LR is at wq_worker_sleeping+0x8/0xc8
[   30.448381] pc : [<c013b8a8>]    lr : [<c01366e0>]    psr: 20000193
[   30.448381] sp : ee1a1c58  ip : eef98f28  fp : ee1a1cb4
[   30.459836] r10: eef98a00  r9 : c0e5f000  r8 : c0d9ea00
[   30.465048] r7 : ee19fcc8  r6 : c0e02e08  r5 : ee19f880  r4 : eef98a00
[   30.471556] r3 : 00000000  r2 : 00000020  r1 : 00000000  r0 : ee19f880
[   30.478066] Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   30.485268] Control: 10c5387d  Table: adcf006a  DAC: 00000051
[   30.490998] Process kworker/1:1 (pid: 71, stack limit = 0xee1a0210)
[   30.497247] Stack: (0xee1a1c58 to 0xee1a2000)
[   30.501590] 1c40:                                                       eef98a00 c0833a68
[   30.509747] 1c60: 00000001 ee117a44 c0d99280 a0000193 00000000 c0833d4c 00000001 ee117a44
[   30.517904] 1c80: 00000000 ee117540 ee19f880 c01faf3c 00000000 ee1a0000 ee1a18ec 00000001
[   30.526060] 1ca0: ee1a1cc8 ee19fc40 c05b0c5c 00000001 ee1a1cc4 c0833d4c ee19f880 ee1a18ec
[   30.534217] 1cc0: ee85d3c0 c012277c ee1a1cc8 ee1a1cc8 00000000 c0e5e2c4 c0e07638 ee1a1e68
[   30.542374] 1ce0: 60000113 0000000b c05b0c5c 00000001 c05b0c5a c010b6f8 ee1a0210 0000000b
[   30.550530] 1d00: c0e07638 ee1a0000 bf000000 00000008 65000000 31313935 20303030 30613365
[   30.558687] 1d20: 34303030 65626520 38623065 35652038 30333439 28203830 33393565 34303033
[   30.566843] 1d40: 00002029 c01b63c4 c0004000 00000004 ee1a1e68 00000017 00000000 00000017
[   30.575000] 1d60: 00000004 eef98580 ee8e3680 c011a424 00000004 c0115c20 ee19f880 00000000
[   30.583156] 1d80: ee19f900 ee2ecd80 ee19f880 c014a55c c014a460 ee19f880 eef98a00 00000000
[   30.591312] 1da0: a0000193 c0e07efc 00000017 c0115888 00000004 ee1a1e68 ee1a0000 eef98580
[   30.599469] 1dc0: ee8e3680 c01012ac 1514622a 00000000 ee19f900 c0149578 eef98a00 ee19f900
[   30.607626] 1de0: 00000001 00000000 eef98a38 00000000 000043e1 c014e11c ee2ecd00 ee19f900
[   30.615781] 1e00: 00000010 00000000 00000006 00000000 00000000 edc45440 c0d992c4 00000000
[   30.623938] 1e20: 00000001 00000000 00000000 00000001 00004b4e 00000000 00000001 c0e5f000
[   30.632095] 1e40: 00000000 00000000 ee19f900 ee19f900 c05b0c58 60000113 ffffffff ee1a1e9c
[   30.640251] 1e60: 00000000 c010bd78 00000001 40000193 00000000 00000000 ee3e03d0 ee3e03f8
[   30.648408] 1e80: c05afef8 ee303000 00000000 00000000 eef98580 ee8e3680 80000000 ee1a1eb8
[   30.656566] 1ea0: c017cda0 c05b0c58 60000113 ffffffff 00000051 00000000 edc0bc00 ee303000
[   30.664722] 1ec0: c05afef8 c05aff2c edc0bc20 c045ec1c eef95c80 eef95c80 00000001 eea82d5c
[   30.672886] 1ee0: edc0bd98 00000000 ee3031c0 ee303218 c0eab740 c05afcc4 ee8e3680 ee3031c0
[   30.681045] 1f00: eef9b800 00000000 eef98580 c0135824 eef98598 c0e02100 eef98580 ee8e3698
[   30.689202] 1f20: 00000008 eef98598 c0e02100 ee1a0000 eef98580 c0135a74 ee840f80 ee8e3680
[   30.697358] 1f40: c0135a3c 00000000 ee840f80 ee8e3680 c0135a3c 00000000 00000000 00000000
[   30.705515] 1f60: 00000000 c013af18 00000000 00000000 00000000 ee8e3680 00000000 00000000
[   30.713672] 1f80: ee1a1f80 ee1a1f80 00000001 00010001 ee1a1f90 ee1a1f90 ee1a1fac ee840f80
[   30.721827] 1fa0: c013ae3c 00000000 00000000 c01078b8 00000000 00000000 00000000 00000000
[   30.729983] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[   30.738140] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
[   30.746304] [<c013b8a8>] (kthread_data) from [<c01366e0>] (wq_worker_sleeping+0x8/0xc8)
[   30.754292] [<c01366e0>] (wq_worker_sleeping) from [<c0833a68>] (__schedule+0x45c/0x6f0)
[   30.762364] [<c0833a68>] (__schedule) from [<c0833d4c>] (schedule+0x50/0xb4)
[   30.769394] [<c0833d4c>] (schedule) from [<c012277c>] (do_exit+0x6e8/0xa90)
[   30.776340] [<c012277c>] (do_exit) from [<c010b6f8>] (die+0x470/0x488)
[   30.782853] [<c010b6f8>] (die) from [<c011a424>] (__do_kernel_fault.part.0+0x64/0x1e4)
[   30.790751] [<c011a424>] (__do_kernel_fault.part.0) from [<c0115c20>] (do_page_fault+0x398/0x3a4)
[   30.799601] [<c0115c20>] (do_page_fault) from [<c01012ac>] (do_DataAbort+0x38/0xb8)
[   30.807238] [<c01012ac>] (do_DataAbort) from [<c010bd78>] (__dabt_svc+0x58/0x80)
[   30.814613] Exception stack(0xee1a1e68 to 0xee1a1eb0)
[   30.819650] 1e60:                   00000001 40000193 00000000 00000000 ee3e03d0 ee3e03f8
[   30.827807] 1e80: c05afef8 ee303000 00000000 00000000 eef98580 ee8e3680 80000000 ee1a1eb8
[   30.835962] 1ea0: c017cda0 c05b0c58 60000113 ffffffff
[   30.841002] [<c010bd78>] (__dabt_svc) from [<c05b0c58>] (sbs_external_power_changed+0x50/0x5c)
[   30.849593] [<c05b0c58>] (sbs_external_power_changed) from [<c05aff2c>] (__power_supply_changed_work+0x34/0x3c)
[   30.859660] [<c05aff2c>] (__power_supply_changed_work) from [<c045ec1c>] (class_for_each_device+0x4c/0xb4)
[   30.869291] [<c045ec1c>] (class_for_each_device) from [<c05afcc4>] (power_supply_changed_work+0x5c/0xb0)
[   30.878762] [<c05afcc4>] (power_supply_changed_work) from [<c0135824>] (process_one_work+0x124/0x33c)
[   30.887963] [<c0135824>] (process_one_work) from [<c0135a74>] (worker_thread+0x38/0x4d4)
[   30.896034] [<c0135a74>] (worker_thread) from [<c013af18>] (kthread+0xdc/0xf4)
[   30.903240] [<c013af18>] (kthread) from [<c01078b8>] (ret_from_fork+0x14/0x3c)
[   30.910443] Code: e34c00a4 ebff90d2 eafffff2 e5903418 (e5130014) 
[   30.916520] ---[ end trace 083d55597e9a2255 ]---
[   30.921122] Fixing recursive fault but reboot is needed!


-- 
nvpublic

[PATCH 2/4] ARM: tegra: nyan: Use external control for bq24735 charger

[Paul Kocialkowski]

Le mercredi 21 septembre 2016 ? 11:10 +0100, Jon Hunter a ?crit?:
> On 21/09/16 08:56, Paul Kocialkowski wrote:
> 
> ...
> 
> > Sure, this is exported at: /sys/class/power_supply/bq24735 at 5-0009
> > Also, note that the power-supply next branch[2] has some more fixes for the
> > bq24735 driver.
> 
> I tested the bq24735 on next-20160919 without any of your changes and?
> when connecting or disconnecting the charger I see the following panic.
> Do you see this?

I have not encountered that, but have been basing my work off the latest rc, not
linux-next.

> / # [???30.120384] Unable to handle kernel NULL pointer dereference at virtual
> address 00000004
> [???30.128489] pgd = c0004000
> [???30.131187] [00000004] *pgd=00000000
> [???30.134759] Internal error: Oops: 17 [#1] PREEMPT SMP ARM
> [???30.140141] Modules linked in:
> [???30.143192] CPU: 1 PID: 71 Comm: kworker/1:1 Not tainted 4.8.0-rc6-next-
> 20160919-00002-gbc9771827865-dirty #574
> [???30.153254] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [???30.159509] Workqueue: events power_supply_changed_work
> [???30.164723] task: ee19f880 task.stack: ee1a0000
> [???30.169239] PC is at sbs_external_power_changed+0x50/0x5c
> [???30.174624] LR is at mod_timer+0x194/0x270
> [???30.178705] pc : [<c05b0c58>]????lr : [<c017cda0>]????psr: 60000113
> [???30.178705] sp : ee1a1eb8??ip : 80000000??fp : ee8e3680
> [???30.190155] r10: eef98580??r9 : 00000000??r8 : 00000000
> [???30.195363] r7 : ee303000??r6 : c05afef8??r5 : ee3e03f8??r4 : ee3e03d0
> [???30.201872] r3 : 00000000??r2 : 00000000??r1 : 40000193??r0 : 00000001
> [???30.208382] Flags: nZCv??IRQs on??FIQs on??Mode SVC_32??ISA ARM??Segment
> none
> [???30.215497] Control: 10c5387d??Table: adcf006a??DAC: 00000051
> [???30.221226] Process kworker/1:1 (pid: 71, stack limit = 0xee1a0210)
> [???30.227474] Stack: (0xee1a1eb8 to 0xee1a2000)
> [???30.231816]
> 1ea0:???????????????????????????????????????????????????????edc0bc00 ee303000
> [???30.239973] 1ec0: c05afef8 c05aff2c edc0bc20 c045ec1c eef95c80 eef95c80
> 00000001 eea82d5c
> [???30.248136] 1ee0: edc0bd98 00000000 ee3031c0 ee303218 c0eab740 c05afcc4
> ee8e3680 ee3031c0
> [???30.256296] 1f00: eef9b800 00000000 eef98580 c0135824 eef98598 c0e02100
> eef98580 ee8e3698
> [???30.264453] 1f20: 00000008 eef98598 c0e02100 ee1a0000 eef98580 c0135a74
> ee840f80 ee8e3680
> [???30.272610] 1f40: c0135a3c 00000000 ee840f80 ee8e3680 c0135a3c 00000000
> 00000000 00000000
> [???30.280766] 1f60: 00000000 c013af18 00000000 00000000 00000000 ee8e3680
> 00000000 00000000
> [???30.288922] 1f80: ee1a1f80 ee1a1f80 00000000 00000000 ee1a1f90 ee1a1f90
> ee1a1fac ee840f80
> [???30.297078] 1fa0: c013ae3c 00000000 00000000 c01078b8 00000000 00000000
> 00000000 00000000
> [???30.305234] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> [???30.313389] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 00000000 00000000
> [???30.321555] [<c05b0c58>] (sbs_external_power_changed) from [<c05aff2c>]
> (__power_supply_changed_work+0x34/0x3c)
> [???30.331623] [<c05aff2c>] (__power_supply_changed_work) from [<c045ec1c>]
> (class_for_each_device+0x4c/0xb4)
> [???30.341254] [<c045ec1c>] (class_for_each_device) from [<c05afcc4>]
> (power_supply_changed_work+0x5c/0xb0)
> [???30.350713] [<c05afcc4>] (power_supply_changed_work) from [<c0135824>]
> (process_one_work+0x124/0x33c)
> [???30.359912] [<c0135824>] (process_one_work) from [<c0135a74>]
> (worker_thread+0x38/0x4d4)
> [???30.367983] [<c0135a74>] (worker_thread) from [<c013af18>]
> (kthread+0xdc/0xf4)
> [???30.375187] [<c013af18>] (kthread) from [<c01078b8>]
> (ret_from_fork+0x14/0x3c)
> [???30.382391] Code: e5911000 e3a00004 ebee0b88 e5943008 (e5933004)?
> [???30.388504] ---[ end trace 083d55597e9a2254 ]---
> [???30.393140] Unable to handle kernel paging request at virtual address
> ffffffec
> [???30.400342] pgd = c0004000
> [???30.403038] [ffffffec] *pgd=afffd861, *pte=00000000, *ppte=00000000
> [???30.409309] Internal error: Oops: 37 [#2] PREEMPT SMP ARM
> [???30.414690] Modules linked in:
> [???30.417738] CPU: 1 PID: 71 Comm: kworker/1:1 Tainted:
> G??????D?????????4.8.0-rc6-next-20160919-00002-gbc9771827865-dirty #574
> [???30.429013] Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> [???30.435265] task: ee19f880 task.stack: ee1a0000
> [???30.439782] PC is at kthread_data+0x4/0xc
> [???30.443779] LR is at wq_worker_sleeping+0x8/0xc8
> [???30.448381] pc : [<c013b8a8>]????lr : [<c01366e0>]????psr: 20000193
> [???30.448381] sp : ee1a1c58??ip : eef98f28??fp : ee1a1cb4
> [???30.459836] r10: eef98a00??r9 : c0e5f000??r8 : c0d9ea00
> [???30.465048] r7 : ee19fcc8??r6 : c0e02e08??r5 : ee19f880??r4 : eef98a00
> [???30.471556] r3 : 00000000??r2 : 00000020??r1 : 00000000??r0 : ee19f880
> [???30.478066] Flags: nzCv??IRQs off??FIQs on??Mode SVC_32??ISA ARM??Segment
> none
> [???30.485268] Control: 10c5387d??Table: adcf006a??DAC: 00000051
> [???30.490998] Process kworker/1:1 (pid: 71, stack limit = 0xee1a0210)
> [???30.497247] Stack: (0xee1a1c58 to 0xee1a2000)
> [???30.501590]
> 1c40:???????????????????????????????????????????????????????eef98a00 c0833a68
> [???30.509747] 1c60: 00000001 ee117a44 c0d99280 a0000193 00000000 c0833d4c
> 00000001 ee117a44
> [???30.517904] 1c80: 00000000 ee117540 ee19f880 c01faf3c 00000000 ee1a0000
> ee1a18ec 00000001
> [???30.526060] 1ca0: ee1a1cc8 ee19fc40 c05b0c5c 00000001 ee1a1cc4 c0833d4c
> ee19f880 ee1a18ec
> [???30.534217] 1cc0: ee85d3c0 c012277c ee1a1cc8 ee1a1cc8 00000000 c0e5e2c4
> c0e07638 ee1a1e68
> [???30.542374] 1ce0: 60000113 0000000b c05b0c5c 00000001 c05b0c5a c010b6f8
> ee1a0210 0000000b
> [???30.550530] 1d00: c0e07638 ee1a0000 bf000000 00000008 65000000 31313935
> 20303030 30613365
> [???30.558687] 1d20: 34303030 65626520 38623065 35652038 30333439 28203830
> 33393565 34303033
> [???30.566843] 1d40: 00002029 c01b63c4 c0004000 00000004 ee1a1e68 00000017
> 00000000 00000017
> [???30.575000] 1d60: 00000004 eef98580 ee8e3680 c011a424 00000004 c0115c20
> ee19f880 00000000
> [???30.583156] 1d80: ee19f900 ee2ecd80 ee19f880 c014a55c c014a460 ee19f880
> eef98a00 00000000
> [???30.591312] 1da0: a0000193 c0e07efc 00000017 c0115888 00000004 ee1a1e68
> ee1a0000 eef98580
> [???30.599469] 1dc0: ee8e3680 c01012ac 1514622a 00000000 ee19f900 c0149578
> eef98a00 ee19f900
> [???30.607626] 1de0: 00000001 00000000 eef98a38 00000000 000043e1 c014e11c
> ee2ecd00 ee19f900
> [???30.615781] 1e00: 00000010 00000000 00000006 00000000 00000000 edc45440
> c0d992c4 00000000
> [???30.623938] 1e20: 00000001 00000000 00000000 00000001 00004b4e 00000000
> 00000001 c0e5f000
> [???30.632095] 1e40: 00000000 00000000 ee19f900 ee19f900 c05b0c58 60000113
> ffffffff ee1a1e9c
> [???30.640251] 1e60: 00000000 c010bd78 00000001 40000193 00000000 00000000
> ee3e03d0 ee3e03f8
> [???30.648408] 1e80: c05afef8 ee303000 00000000 00000000 eef98580 ee8e3680
> 80000000 ee1a1eb8
> [???30.656566] 1ea0: c017cda0 c05b0c58 60000113 ffffffff 00000051 00000000
> edc0bc00 ee303000
> [???30.664722] 1ec0: c05afef8 c05aff2c edc0bc20 c045ec1c eef95c80 eef95c80
> 00000001 eea82d5c
> [???30.672886] 1ee0: edc0bd98 00000000 ee3031c0 ee303218 c0eab740 c05afcc4
> ee8e3680 ee3031c0
> [???30.681045] 1f00: eef9b800 00000000 eef98580 c0135824 eef98598 c0e02100
> eef98580 ee8e3698
> [???30.689202] 1f20: 00000008 eef98598 c0e02100 ee1a0000 eef98580 c0135a74
> ee840f80 ee8e3680
> [???30.697358] 1f40: c0135a3c 00000000 ee840f80 ee8e3680 c0135a3c 00000000
> 00000000 00000000
> [???30.705515] 1f60: 00000000 c013af18 00000000 00000000 00000000 ee8e3680
> 00000000 00000000
> [???30.713672] 1f80: ee1a1f80 ee1a1f80 00000001 00010001 ee1a1f90 ee1a1f90
> ee1a1fac ee840f80
> [???30.721827] 1fa0: c013ae3c 00000000 00000000 c01078b8 00000000 00000000
> 00000000 00000000
> [???30.729983] 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
> [???30.738140] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
> 00000000 00000000
> [???30.746304] [<c013b8a8>] (kthread_data) from [<c01366e0>]
> (wq_worker_sleeping+0x8/0xc8)
> [???30.754292] [<c01366e0>] (wq_worker_sleeping) from [<c0833a68>]
> (__schedule+0x45c/0x6f0)
> [???30.762364] [<c0833a68>] (__schedule) from [<c0833d4c>]
> (schedule+0x50/0xb4)
> [???30.769394] [<c0833d4c>] (schedule) from [<c012277c>] (do_exit+0x6e8/0xa90)
> [???30.776340] [<c012277c>] (do_exit) from [<c010b6f8>] (die+0x470/0x488)
> [???30.782853] [<c010b6f8>] (die) from [<c011a424>]
> (__do_kernel_fault.part.0+0x64/0x1e4)
> [???30.790751] [<c011a424>] (__do_kernel_fault.part.0) from [<c0115c20>]
> (do_page_fault+0x398/0x3a4)
> [???30.799601] [<c0115c20>] (do_page_fault) from [<c01012ac>]
> (do_DataAbort+0x38/0xb8)
> [???30.807238] [<c01012ac>] (do_DataAbort) from [<c010bd78>]
> (__dabt_svc+0x58/0x80)
> [???30.814613] Exception stack(0xee1a1e68 to 0xee1a1eb0)
> [???30.819650] 1e60:???????????????????00000001 40000193 00000000 00000000
> ee3e03d0 ee3e03f8
> [???30.827807] 1e80: c05afef8 ee303000 00000000 00000000 eef98580 ee8e3680
> 80000000 ee1a1eb8
> [???30.835962] 1ea0: c017cda0 c05b0c58 60000113 ffffffff
> [???30.841002] [<c010bd78>] (__dabt_svc) from [<c05b0c58>]
> (sbs_external_power_changed+0x50/0x5c)
> [???30.849593] [<c05b0c58>] (sbs_external_power_changed) from [<c05aff2c>]
> (__power_supply_changed_work+0x34/0x3c)
> [???30.859660] [<c05aff2c>] (__power_supply_changed_work) from [<c045ec1c>]
> (class_for_each_device+0x4c/0xb4)
> [???30.869291] [<c045ec1c>] (class_for_each_device) from [<c05afcc4>]
> (power_supply_changed_work+0x5c/0xb0)
> [???30.878762] [<c05afcc4>] (power_supply_changed_work) from [<c0135824>]
> (process_one_work+0x124/0x33c)
> [???30.887963] [<c0135824>] (process_one_work) from [<c0135a74>]
> (worker_thread+0x38/0x4d4)
> [???30.896034] [<c0135a74>] (worker_thread) from [<c013af18>]
> (kthread+0xdc/0xf4)
> [???30.903240] [<c013af18>] (kthread) from [<c01078b8>]
> (ret_from_fork+0x14/0x3c)
> [???30.910443] Code: e34c00a4 ebff90d2 eafffff2 e5903418 (e5130014)?
> [???30.916520] ---[ end trace 083d55597e9a2255 ]---
> [???30.921122] Fixing recursive fault but reboot is needed!
> 
> 
-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/e1152088/attachment.sig>

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Paul Kocialkowski]

Depthcharge (the payload used with cros devices) will attempt to detect
boards using their revision. This includes all the known revisions for
the nyan-big board so that the dtb can be selected preferably.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/tegra124-nyan-big.dts | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124-nyan-big.dts b/arch/arm/boot/dts/tegra124-nyan-big.dts
index 67d7cfb..f12ece9 100644
--- a/arch/arm/boot/dts/tegra124-nyan-big.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-big.dts
@@ -6,7 +6,12 @@
 
 / {
 	model = "Acer Chromebook 13 CB5-311";
-	compatible = "google,nyan-big", "nvidia,tegra124";
+	compatible = "google,nyan-big-rev7", "google,nyan-big-rev6",
+			"google,nyan-big-rev5", "google,nyan-big-rev4",
+			"google,nyan-big-rev3", "google,nyan-big-rev2",
+			"google,nyan-big-rev5", "google,nyan-big-rev4",
+			"google,nyan-big-rev1", "google,nyan-big-rev0",
+			"google,nyan-big", "google,nyan", "nvidia,tegra124";
 
 	panel: panel {
 		compatible = "auo,b133xtn01";
-- 
2.9.3

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Jon Hunter]

On 28/08/16 18:32, Paul Kocialkowski wrote:
> Depthcharge (the payload used with cros devices) will attempt to detect
> boards using their revision. This includes all the known revisions for
> the nyan-big board so that the dtb can be selected preferably.

May be I am missing something here, but for the mainline there is only
one dtb available and so why is this needed for the mainline?

I understand for the downstream kernels that the chromebooks ship with
they may include multiple dtbs, but for the mainline we only have one.

Cheers
Jon

-- 
nvpublic

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Paul Kocialkowski]

Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit?:
> On 28/08/16 18:32, Paul Kocialkowski wrote:
> > 
> > Depthcharge (the payload used with cros devices) will attempt to detect
> > boards using their revision. This includes all the known revisions for
> > the nyan-big board so that the dtb can be selected preferably.
> 
> May be I am missing something here, but for the mainline there is only
> one dtb available and so why is this needed for the mainline?

There is indeed a single dts in mainline, but depthcharge will use the revision
to match the compatible string (e.g. it will look for google,nyan-big-rev5, not
google,nyan-big), so we need to list them all in that single dts. Otherwise,
depthcharge will fall back to the default config, which may or may not be
suitable for nyan.

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/30d1d305/attachment-0001.sig>

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Jon Hunter]

On 20/09/16 18:53, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit :
>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>
>>> Depthcharge (the payload used with cros devices) will attempt to detect
>>> boards using their revision. This includes all the known revisions for
>>> the nyan-big board so that the dtb can be selected preferably.
>>
>> May be I am missing something here, but for the mainline there is only
>> one dtb available and so why is this needed for the mainline?
> 
> There is indeed a single dts in mainline, but depthcharge will use the revision
> to match the compatible string (e.g. it will look for google,nyan-big-rev5, not
> google,nyan-big), so we need to list them all in that single dts. Otherwise,
> depthcharge will fall back to the default config, which may or may not be
> suitable for nyan.

Is tegra124-nyan-big.dtb not the default?

Jon

-- 
nvpublic

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Paul Kocialkowski]

Le mardi 20 septembre 2016 ? 18:56 +0100, Jon Hunter a ?crit?:
> On 20/09/16 18:53, Paul Kocialkowski wrote:
> > 
> > * PGP Signed by an unknown key
> > 
> > Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit :
> > > 
> > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > 
> > > > 
> > > > Depthcharge (the payload used with cros devices) will attempt to detect
> > > > boards using their revision. This includes all the known revisions for
> > > > the nyan-big board so that the dtb can be selected preferably.
> > > 
> > > May be I am missing something here, but for the mainline there is only
> > > one dtb available and so why is this needed for the mainline?
> > 
> > There is indeed a single dts in mainline, but depthcharge will use the
> > revision
> > to match the compatible string (e.g. it will look for google,nyan-big-rev5,
> > not
> > google,nyan-big), so we need to list them all in that single dts. Otherwise,
> > depthcharge will fall back to the default config, which may or may not be
> > suitable for nyan.
> 
> Is tegra124-nyan-big.dtb not the default?

You can't expect that to always be the case. The image format allows many
different dts to be provided, so I could easily build with multi_v7_defconfig
and have various dts for various devices in the same image, and just select a
random one as default.

Here, default is really a fallback, the right one is expected to be detected by
this mechanism. And it really doesn't hurt to provide that information for
proper detection.

Note that this is done with many other cros devices in mainline (such as rk3288
veyrons).

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/82f5dcd1/attachment.sig>

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Jon Hunter]

On 20/09/16 19:02, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mardi 20 septembre 2016 ? 18:56 +0100, Jon Hunter a ?crit :
>> On 20/09/16 18:53, Paul Kocialkowski wrote:
>>>
>>>> Old Signed by an unknown key
>>>
>>> Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit :
>>>>
>>>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>>>
>>>>>
>>>>> Depthcharge (the payload used with cros devices) will attempt to detect
>>>>> boards using their revision. This includes all the known revisions for
>>>>> the nyan-big board so that the dtb can be selected preferably.
>>>>
>>>> May be I am missing something here, but for the mainline there is only
>>>> one dtb available and so why is this needed for the mainline?
>>>
>>> There is indeed a single dts in mainline, but depthcharge will use the
>>> revision
>>> to match the compatible string (e.g. it will look for google,nyan-big-rev5,
>>> not
>>> google,nyan-big), so we need to list them all in that single dts. Otherwise,
>>> depthcharge will fall back to the default config, which may or may not be
>>> suitable for nyan.
>>
>> Is tegra124-nyan-big.dtb not the default?
> 
> You can't expect that to always be the case. The image format allows many
> different dts to be provided, so I could easily build with multi_v7_defconfig
> and have various dts for various devices in the same image, and just select a
> random one as default.

Really? Sounds odd. I was hoping that tegra124-nyan-big.dtb would be a
catch all.

> Here, default is really a fallback, the right one is expected to be detected by
> this mechanism. And it really doesn't hurt to provide that information for
> proper detection.
> 
> Note that this is done with many other cros devices in mainline (such as rk3288
> veyrons).

Yes, I guess the same is true for the tegra210-smaug and we do define
all the revs for that one. May be extend the changelog comment to say
what may happen without this change and the problem that this is fixing.

Jon	

-- 
nvpublic

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Paul Kocialkowski]

Le mercredi 21 septembre 2016 ? 08:34 +0100, Jon Hunter a ?crit?:
> On 20/09/16 19:02, Paul Kocialkowski wrote:
> > 
> > * PGP Signed by an unknown key
> > 
> > Le mardi 20 septembre 2016 ? 18:56 +0100, Jon Hunter a ?crit :
> > > 
> > > On 20/09/16 18:53, Paul Kocialkowski wrote:
> > > > 
> > > > 
> > > > > 
> > > > > Old Signed by an unknown key
> > > > 
> > > > Le mardi 20 septembre 2016 ? 18:41 +0100, Jon Hunter a ?crit :
> > > > > 
> > > > > 
> > > > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > Depthcharge (the payload used with cros devices) will attempt to
> > > > > > detect
> > > > > > boards using their revision. This includes all the known revisions
> > > > > > for
> > > > > > the nyan-big board so that the dtb can be selected preferably.
> > > > > 
> > > > > May be I am missing something here, but for the mainline there is only
> > > > > one dtb available and so why is this needed for the mainline?
> > > > 
> > > > There is indeed a single dts in mainline, but depthcharge will use the
> > > > revision
> > > > to match the compatible string (e.g. it will look for google,nyan-big-
> > > > rev5,
> > > > not
> > > > google,nyan-big), so we need to list them all in that single dts.
> > > > Otherwise,
> > > > depthcharge will fall back to the default config, which may or may not
> > > > be
> > > > suitable for nyan.
> > > 
> > > Is tegra124-nyan-big.dtb not the default?
> > 
> > You can't expect that to always be the case. The image format allows many
> > different dts to be provided, so I could easily build with
> > multi_v7_defconfig
> > and have various dts for various devices in the same image, and just select
> > a
> > random one as default.
> 
> Really? Sounds odd. I was hoping that tegra124-nyan-big.dtb would be a
> catch all.

Yes, the image format (FIT) allows specifying multiple dtb and zImage
combinations in the same image[0].

See depthcharge[|1] for how the "compatible" property is being matched against.

> > Here, default is really a fallback, the right one is expected to be detected
> > by
> > this mechanism. And it really doesn't hurt to provide that information for
> > proper detection.
> > 
> > Note that this is done with many other cros devices in mainline (such as
> > rk3288
> > veyrons).
> 
> Yes, I guess the same is true for the tegra210-smaug and we do define
> all the revs for that one. May be extend the changelog comment to say
> what may happen without this change and the problem that this is fixing.

Allright, will do!

Thanks

[0]:?http://www.denx.de/wiki/pub/U-Boot/Documentation/multi_image_booting_scenarios.pdf
[1]:?https://chromium.googlesource.com/chromiumos/platform/depthcharge/+/master/src/boot/fit.c#39

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/d16c5577/attachment.sig>

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Jon Hunter]

On 21/09/16 08:43, Paul Kocialkowski wrote:

...

>>>>>>> Depthcharge (the payload used with cros devices) will attempt to
>>>>>>> detect
>>>>>>> boards using their revision. This includes all the known revisions
>>>>>>> for
>>>>>>> the nyan-big board so that the dtb can be selected preferably.
>>>>>>
>>>>>> May be I am missing something here, but for the mainline there is only
>>>>>> one dtb available and so why is this needed for the mainline?
>>>>>
>>>>> There is indeed a single dts in mainline, but depthcharge will use the
>>>>> revision
>>>>> to match the compatible string (e.g. it will look for google,nyan-big-
>>>>> rev5,
>>>>> not
>>>>> google,nyan-big), so we need to list them all in that single dts.
>>>>> Otherwise,
>>>>> depthcharge will fall back to the default config, which may or may not
>>>>> be
>>>>> suitable for nyan.
>>>>
>>>> Is tegra124-nyan-big.dtb not the default?
>>>
>>> You can't expect that to always be the case. The image format allows many
>>> different dts to be provided, so I could easily build with
>>> multi_v7_defconfig
>>> and have various dts for various devices in the same image, and just select
>>> a
>>> random one as default.
>>
>> Really? Sounds odd. I was hoping that tegra124-nyan-big.dtb would be a
>> catch all.

I meant I was hoping that compatible = "google,nyan-big" would be the
catchall not the dtb file name ;-)

> Yes, the image format (FIT) allows specifying multiple dtb and zImage
> combinations in the same image[0].

Yes I am aware of that. Typically, I have been testing using a FIT image
with single zImage and dtb. Hence no problems.

So are you wanting to create a FIT image to support multiple boards and
use the single FIT image for all? If so then I can see why you want
this. Again please describe the motivation for the changes in the
changelog so it is clear why we are adding this.

Cheers
Jon

-- 
nvpublic

[PATCH 3/4] ARM: tegra: nyan-big: Include compatible revisions for proper detection

[Paul Kocialkowski]

Resending with the right CC chain.

Le mercredi 21 septembre 2016 ? 10:15 +0100, Jon Hunter a ?crit?:
> 
> On 21/09/16 08:43, Paul Kocialkowski wrote:
> 
> ...
> 
> > > > > > > > Depthcharge (the payload used with cros devices) will attempt to
> > > > > > > > detect
> > > > > > > > boards using their revision. This includes all the known
> > > > > > > > revisions
> > > > > > > > for
> > > > > > > > the nyan-big board so that the dtb can be selected preferably.
> > > > > > > 
> > > > > > > May be I am missing something here, but for the mainline there is
> > > > > > > only
> > > > > > > one dtb available and so why is this needed for the mainline?
> > > > > > 
> > > > > > There is indeed a single dts in mainline, but depthcharge will use
> > > > > > the
> > > > > > revision
> > > > > > to match the compatible string (e.g. it will look for google,nyan-
> > > > > > big-
> > > > > > rev5,
> > > > > > not
> > > > > > google,nyan-big), so we need to list them all in that single dts.
> > > > > > Otherwise,
> > > > > > depthcharge will fall back to the default config, which may or may
> > > > > > not
> > > > > > be
> > > > > > suitable for nyan.
> > > > > 
> > > > > Is tegra124-nyan-big.dtb not the default?
> > > > 
> > > > You can't expect that to always be the case. The image format allows
> > > > many
> > > > different dts to be provided, so I could easily build with
> > > > multi_v7_defconfig
> > > > and have various dts for various devices in the same image, and just
> > > > select
> > > > a
> > > > random one as default.
> > > 
> > > Really? Sounds odd. I was hoping that tegra124-nyan-big.dtb would be a
> > > catch all.
> 
> I meant I was hoping that compatible = "google,nyan-big" would be the
> catchall not the dtb file name ;-)

Yeah, I figured :)

> 
> > 
> > Yes, the image format (FIT) allows specifying multiple dtb and zImage
> > combinations in the same image[0].
> 
> Yes I am aware of that. Typically, I have been testing using a FIT image
> with single zImage and dtb. Hence no problems.
> 
> So are you wanting to create a FIT image to support multiple boards and
> use the single FIT image for all? If so then I can see why you want
> this. Again please describe the motivation for the changes in the
> changelog so it is clear why we are adding this.

Fair enough, will do in v2.

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/3e5c3e2d/attachment.sig>

[PATCH 4/4] ARM: tegra: nyan-blaze: Include compatible revisions for proper detection

[Paul Kocialkowski]

Depthcharge (the payload used with cros devices) will attempt to detect
boards using their revision. This includes all the known revisions for
the nyan-blaze board so that the dtb can be selected preferably.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 arch/arm/boot/dts/tegra124-nyan-blaze.dts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra124-nyan-blaze.dts b/arch/arm/boot/dts/tegra124-nyan-blaze.dts
index c9582361..aca0828 100644
--- a/arch/arm/boot/dts/tegra124-nyan-blaze.dts
+++ b/arch/arm/boot/dts/tegra124-nyan-blaze.dts
@@ -6,7 +6,13 @@
 
 / {
 	model = "HP Chromebook 14";
-	compatible = "google,nyan-blaze", "google,nyan", "nvidia,tegra124";
+	compatible = "google,nyan-blaze-rev10", "google,nyan-blaze-rev9",
+			"google,nyan-blaze-rev8", "google,nyan-blaze-rev7",
+			"google,nyan-blaze-rev6", "google,nyan-blaze-rev5",
+			"google,nyan-blaze-rev4", "google,nyan-blaze-rev3",
+			"google,nyan-blaze-rev2", "google,nyan-blaze-rev1",
+			"google,nyan-blaze-rev0", "google,nyan-blaze",
+			"google,nyan", "nvidia,tegra124";
 
 	panel: panel {
 		compatible = "samsung,ltn140at29-301";
-- 
2.9.3

[PATCH 4/4] ARM: tegra: nyan-blaze: Include compatible revisions for proper detection

[Jon Hunter]

On 28/08/16 18:32, Paul Kocialkowski wrote:
> Depthcharge (the payload used with cros devices) will attempt to detect
> boards using their revision. This includes all the known revisions for
> the nyan-blaze board so that the dtb can be selected preferably.

Again I am not sure what the benefit/need for this is.

Jon

-- 
nvpublic

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Jon Hunter]

On 28/08/16 18:32, Paul Kocialkowski wrote:
> This switches a few interrupt definitions that were using
> GPIO_ACTIVE_HIGH as IRQ type, which is invalid.

May be you are right, but this does not describe why this is invalid.
Can you elaborate?

> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
> index 271505e..30a77ec 100644
> --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> @@ -59,7 +59,7 @@
>  			compatible = "maxim,max98090";
>  			reg = <0x10>;
>  			interrupt-parent = <&gpio>;
> -			interrupts = <TEGRA_GPIO(H, 4) GPIO_ACTIVE_HIGH>;
> +			interrupts = <TEGRA_GPIO(H, 4) IRQ_TYPE_EDGE_FALLING>;

If this in invalid then the DT binding doc for the max98090 should be
updated as well.

>  		};
>  
>  		temperature-sensor at 4c {
> @@ -325,7 +325,7 @@
>  					reg = <0x9>;
>  					interrupt-parent = <&gpio>;
>  					interrupts = <TEGRA_GPIO(J, 0)
> -							GPIO_ACTIVE_HIGH>;
> +							IRQ_TYPE_EDGE_BOTH>;
>  					ti,ac-detect-gpios = <&gpio
>  							TEGRA_GPIO(J, 0)
>  							GPIO_ACTIVE_HIGH>;
> 

Cheers
Jon

-- 
nvpublic

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Paul Kocialkowski]

Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit?:
> On 28/08/16 18:32, Paul Kocialkowski wrote:
> > 
> > This switches a few interrupt definitions that were using
> > GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
> 
> May be you are right, but this does not describe why this is invalid.
> Can you elaborate?

GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
"interrupts" devicetree property. Values provided there are understood as
IRQ_TYPE_ defines.

Also, a similar commit[0] was pushed to the cros kernel tree.

Cheers,

[0]:?https://chromium.googlesource.com/chromiumos/third_party/kernel/+/f77288938d571b48c9736ac3703cea370c002434

> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> > ?arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++--
> > ?1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > index 271505e..30a77ec 100644
> > --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > @@ -59,7 +59,7 @@
> > ?			compatible = "maxim,max98090";
> > ?			reg = <0x10>;
> > ?			interrupt-parent = <&gpio>;
> > -			interrupts = <TEGRA_GPIO(H, 4) GPIO_ACTIVE_HIGH>;
> > +			interrupts = <TEGRA_GPIO(H, 4)
> > IRQ_TYPE_EDGE_FALLING>;
> 
> If this in invalid then the DT binding doc for the max98090 should be
> updated as well.
> 
> > 
> > ?		};
> > ?
> > ?		temperature-sensor at 4c {
> > @@ -325,7 +325,7 @@
> > ?					reg = <0x9>;
> > ?					interrupt-parent = <&gpio>;
> > ?					interrupts = <TEGRA_GPIO(J, 0)
> > -							GPIO_ACTIVE_HIGH>;
> > +							IRQ_TYPE_EDGE_BOTH>
> > ;
> > ?					ti,ac-detect-gpios = <&gpio
> > ?							TEGRA_GPIO(J, 0)
> > ?							GPIO_ACTIVE_HIGH>;
> > 
> 
> Cheers
> Jon
> 
-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160920/cc2229e4/attachment.sig>

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Jon Hunter]

On 20/09/16 19:14, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit :
>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>
>>> This switches a few interrupt definitions that were using
>>> GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
>>
>> May be you are right, but this does not describe why this is invalid.
>> Can you elaborate?
> 
> GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
> "interrupts" devicetree property. Values provided there are understood as
> IRQ_TYPE_ defines.

Right, but you are changing the type as GPIO_ACTIVE_HIGH = 0 and
IRQ_TYPE_EDGE_FALLING = 2 and there is no comment about why this has
been changed. It might be correct, but you need to explain it.

Jon

-- 
nvpublic

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Paul Kocialkowski]

Le mercredi 21 septembre 2016 ? 08:52 +0100, Jon Hunter a ?crit?:
> On 20/09/16 19:14, Paul Kocialkowski wrote:
> > 
> > * PGP Signed by an unknown key
> > 
> > Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit :
> > > 
> > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > 
> > > > 
> > > > This switches a few interrupt definitions that were using
> > > > GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
> > > 
> > > May be you are right, but this does not describe why this is invalid.
> > > Can you elaborate?
> > 
> > GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
> > "interrupts" devicetree property. Values provided there are understood as
> > IRQ_TYPE_ defines.
> 
> Right, but you are changing the type as GPIO_ACTIVE_HIGH = 0 and
> IRQ_TYPE_EDGE_FALLING = 2 and there is no comment about why this has
> been changed. It might be correct, but you need to explain it.

This actually makes the IRQ trigger values consistent with the drivers, that
define them regardless of devicetree anyway. The max98090 driver
has?IRQF_TRIGGER_FALLING and bq24735 has IRQF_TRIGGER_RISING |
IRQF_TRIGGER_FALLING.

This is really more of a cosmetic change, it doesn't impact actual use.

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/989111ff/attachment.sig>

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Jon Hunter]

On 21/09/16 09:26, Paul Kocialkowski wrote:
> * PGP Signed by an unknown key
> 
> Le mercredi 21 septembre 2016 ? 08:52 +0100, Jon Hunter a ?crit :
>> On 20/09/16 19:14, Paul Kocialkowski wrote:
>>>
>>>> Old Signed by an unknown key
>>>
>>> Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit :
>>>>
>>>> On 28/08/16 18:32, Paul Kocialkowski wrote:
>>>>>
>>>>>
>>>>> This switches a few interrupt definitions that were using
>>>>> GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
>>>>
>>>> May be you are right, but this does not describe why this is invalid.
>>>> Can you elaborate?
>>>
>>> GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
>>> "interrupts" devicetree property. Values provided there are understood as
>>> IRQ_TYPE_ defines.
>>
>> Right, but you are changing the type as GPIO_ACTIVE_HIGH = 0 and
>> IRQ_TYPE_EDGE_FALLING = 2 and there is no comment about why this has
>> been changed. It might be correct, but you need to explain it.
> 
> This actually makes the IRQ trigger values consistent with the drivers, that
> define them regardless of devicetree anyway. The max98090 driver
> has IRQF_TRIGGER_FALLING and bq24735 has IRQF_TRIGGER_RISING |
> IRQF_TRIGGER_FALLING.
> 
> This is really more of a cosmetic change, it doesn't impact actual use.

So you are saying that the drivers don't actually use the DT types? May
be that is ok, and yes this is cosmetic, but this should be stated in
the changelog as it is not clear what is going on here.

Cheers
Jon

-- 
nvpublic

[PATCH 1/4] ARM: tegra: nyan: Use proper IRQ type definitions

[Paul Kocialkowski]

Resending with the right CC chain.

Le mercredi 21 septembre 2016 ? 10:06 +0100, Jon Hunter a ?crit?:
> On 21/09/16 09:26, Paul Kocialkowski wrote:
> > 
> > 
> > * PGP Signed by an unknown key
> > 
> > Le mercredi 21 septembre 2016 ? 08:52 +0100, Jon Hunter a ?crit :
> > > 
> > > 
> > > On 20/09/16 19:14, Paul Kocialkowski wrote:
> > > > 
> > > > 
> > > > 
> > > > > 
> > > > > 
> > > > > Old Signed by an unknown key
> > > > 
> > > > Le mardi 20 septembre 2016 ? 18:15 +0100, Jon Hunter a ?crit :
> > > > > 
> > > > > 
> > > > > 
> > > > > On 28/08/16 18:32, Paul Kocialkowski wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > This switches a few interrupt definitions that were using
> > > > > > GPIO_ACTIVE_HIGH as IRQ type, which is invalid.
> > > > > 
> > > > > May be you are right, but this does not describe why this is invalid.
> > > > > Can you elaborate?
> > > > 
> > > > GPIO_ACTIVE_HIGH is simply not the right kind of define to use in the
> > > > "interrupts" devicetree property. Values provided there are understood
> > > > as
> > > > IRQ_TYPE_ defines.
> > > 
> > > Right, but you are changing the type as GPIO_ACTIVE_HIGH = 0 and
> > > IRQ_TYPE_EDGE_FALLING = 2 and there is no comment about why this has
> > > been changed. It might be correct, but you need to explain it.
> > 
> > This actually makes the IRQ trigger values consistent with the drivers, that
> > define them regardless of devicetree anyway. The max98090 driver
> > has IRQF_TRIGGER_FALLING and bq24735 has IRQF_TRIGGER_RISING |
> > IRQF_TRIGGER_FALLING.
> > 
> > This is really more of a cosmetic change, it doesn't impact actual use.
> 
> So you are saying that the drivers don't actually use the DT types?

It appears so. At least they are hardcoded in the kernel driver.

> 
> ?May be that is ok, and yes this is cosmetic, but this should be stated in
> the changelog as it is not clear what is going on here.

Fair enough, will do in v2.

-- 
Paul Kocialkowski, developer of low-level free software for embedded devices

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160921/c34cbaae/attachment-0001.sig>