POE HAT driver cleanup #2638

XECDesign · 2018-08-06T13:47:38Z

Signed-off-by: Serge Schneider [email protected]

lategoodbye · 2018-08-10T09:17:23Z

I will add some suggestions to make upstreaming easier.

pelwell · 2018-08-10T09:39:41Z

Merged on the command line, with added README entry for the overlay.

@lategoodbye Upstreaming suggestions will still be gratefully received.

lategoodbye · 2018-08-10T09:21:23Z

drivers/hwmon/Kconfig

+	depends on THERMAL || THERMAL=n
+	help
+	  If you say yes here you get support for Raspberry Pi POE HAT fan.
+


It would be nice to write down the meaning of the acronym at least in this help (Power over Ethernet).

lategoodbye · 2018-08-10T09:22:14Z

drivers/hwmon/rpi-poe-fan.c

@@ -0,0 +1,443 @@
+/*


Please add SPDX tag for new drivers. So you can drop the license text below.

I'd never heard of SPDX! Had to look it up. Do you mean adding this string?

SPDX-License-Identifier: GPL-2.0

Yes. In general it must be a C++ comment.

lategoodbye · 2018-08-10T09:24:23Z

drivers/hwmon/rpi-poe-fan.c

+	struct notifier_block nb;
+};
+
+struct m_data_s{


I think this struct name is not self-explaining.

Would fw_tag_data be clear?

lategoodbye · 2018-08-10T09:27:47Z

drivers/hwmon/rpi-poe-fan.c

+
+	ret = of_property_count_u32_elems(np, "cooling-levels");
+	if (ret <= 0) {
+		dev_err(dev, "Wrong data!\n");


Please provide a helpful error message, like cooling-levels have wrong format or something else.

lategoodbye · 2018-08-10T09:30:52Z

drivers/hwmon/rpi-poe-fan.c

+			 rpi_poe_fan_resume);
+
+static const struct of_device_id of_rpi_poe_fan_match[] = {
+	{ .compatible = "rpi-poe-fan", },


The vendor name is missing. Please use "raspberrypi,rpi-poe-fan"

The vendor part should also go in the dt overlay? Or is that okay as just "rpi-poe-fan"?

Yes, and no, respectively - they must match.

lategoodbye · 2018-08-10T09:33:46Z

include/soc/bcm2835/raspberrypi-firmware.h

@@ -93,6 +93,8 @@ enum rpi_firmware_property_tag {
 	RPI_FIRMWARE_SET_GPIO_CONFIG =                        0x00038043,
 	RPI_FIRMWARE_GET_PERIPH_REG =                         0x00030045,
 	RPI_FIRMWARE_SET_PERIPH_REG =                         0x00038045,
+	RPI_FIRMWARE_GET_POE_HAT_VAL =                        0x00030049,
+	RPI_FIRMWARE_SET_POE_HAT_VAL =                        0x00030050,


Out of curiosity, is there any chance to read back the revolutions per minute from the fan?

It's just a simple fan switched by PWM on an attiny814. No way to read back the RPM.

lategoodbye · 2018-08-10T09:36:21Z

arch/arm/boot/dts/overlays/rpi-poe-overlay.dts

+					temperature = <55000>;
+					hysteresis = <2000>;
+					type = "active";
+				};


Are these trip points Raspberry Pi 3B+ specific?

They're mostly arbitrary with the intention of keeping the temperature below 60c. That point was chosen because the 3B+ scales down to 1.2GHz there. I haven't given other models much thought, since the transformer centre tap pins are only available on the 3B+. If somebody wanted to use the HAT just for the fan on another model, I dont know if they would want to be able to change those points.

lategoodbye · 2018-08-10T09:46:38Z

arch/arm/boot/dts/overlays/rpi-poe-overlay.dts

+				status = "okay";
+			};
+		};
+	};


It would be nice to have a binding document for this driver like Documentation/devicetree/bindings/hwmon/pwm-fan.txt

and a hwmon documentation like Documentation/hwmon/pwm-fan

See: raspberrypi/linux#2638 firmware: Add POE HAT support

XECDesign · 2018-08-28T09:42:02Z

Thanks for the suggestions, @lategoodbye.

Updated to address those and also fix issue #2665

~~Hasn't been built and tested yet.~~

lassebm · 2018-08-28T17:03:48Z

Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt

@@ -0,0 +1,55 @@
+Bindings for the Raspberry Pi POE HAT fan


s/POE/PoE/. Same elsewhere.

lassebm · 2018-08-28T17:07:12Z

Documentation/hwmon/rpi-poe-fan

+-----------
+
+The driver implements a simple interface for driving the Raspberry Pi POE
+(Power over Ethernet) HAT fan. The driver passes commands to the Raspberry pi


lassebm · 2018-08-28T17:08:19Z

Documentation/hwmon/rpi-poe-fan

+(Power over Ethernet) HAT fan. The driver passes commands to the Raspberry pi
+firmware through the mailbox property interface. The firmware then forwards
+the commands to the board over I2C on the ID_EEPROM pins. The driver exposes
+the fan to the user space through the hwmon's sysfs interface.


s/hwmon's/hwmon/.

closes raspberrypi#2665 Signed-off-by: Serge Schneider <[email protected]>

Signed-off-by: Serge Schneider <[email protected]>

…s fail Signed-off-by: Serge Schneider <[email protected]>

Signed-off-by: Serge Schneider <[email protected]>

XECDesign · 2018-08-29T14:00:58Z

Updated.

Thanks, @lassebm.

kernel: Add support for audioinjector.net ultra soundcard See: raspberrypi/linux#2664 kernel: POE HAT driver cleanup See: raspberrypi/linux#2638 firmware: arm_dt: Work around an absent cache-line-size See: raspberrypi/linux#2643

Emergency stack path was jumping into a 3: label inside the __GEN_COMMON_BODY macro for the normal path after it had finished, rather than jumping over it. By a small miracle this is the correct place to build up a new interrupt frame with the existing stack pointer, so things basically worked okay with an added weird looking 700 trap frame on top (which had the wrong ->nip so it didn't decode bug messages either). Fix this by avoiding using numeric labels when jumping over non-trivial macros. Before: LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV Modules linked in: CPU: 0 PID: 88 Comm: sh Not tainted 5.15.0-rc2-00034-ge057cdade6e5 #2637 NIP: 7265677368657265 LR: c00000000006c0c8 CTR: c0000000000097f0 REGS: c0000000fffb3a50 TRAP: 0700 Not tainted MSR: 9000000000021031 <SF,HV,ME,IR,DR,LE> CR: 00000700 XER: 20040000 CFAR: c0000000000098b0 IRQMASK: 0 GPR00: c00000000006c964 c0000000fffb3cf0 c000000001513800 0000000000000000 GPR04: 0000000048ab0778 0000000042000000 0000000000000000 0000000000001299 GPR08: 000001e447c718ec 0000000022424282 0000000000002710 c00000000006bee8 GPR12: 9000000000009033 c0000000016b0000 00000000000000b0 0000000000000001 GPR16: 0000000000000000 0000000000000002 0000000000000000 0000000000000ff8 GPR20: 0000000000001fff 0000000000000007 0000000000000080 00007fff89d90158 GPR24: 0000000002000000 0000000002000000 0000000000000255 0000000000000300 GPR28: c000000001270000 0000000042000000 0000000048ab0778 c000000080647e80 NIP [7265677368657265] 0x7265677368657265 LR [c00000000006c0c8] ___do_page_fault+0x3f8/0xb10 Call Trace: [c0000000fffb3cf0] [c00000000000bdac] soft_nmi_common+0x13c/0x1d0 (unreliable) --- interrupt: 700 at decrementer_common_virt+0xb8/0x230 NIP: c0000000000098b8 LR: c00000000006c0c8 CTR: c0000000000097f0 REGS: c0000000fffb3d60 TRAP: 0700 Not tainted MSR: 9000000000021031 <SF,HV,ME,IR,DR,LE> CR: 22424282 XER: 20040000 CFAR: c0000000000098b0 IRQMASK: 0 GPR00: c00000000006c964 0000000000002400 c000000001513800 0000000000000000 GPR04: 0000000048ab0778 0000000042000000 0000000000000000 0000000000001299 GPR08: 000001e447c718ec 0000000022424282 0000000000002710 c00000000006bee8 GPR12: 9000000000009033 c0000000016b0000 00000000000000b0 0000000000000001 GPR16: 0000000000000000 0000000000000002 0000000000000000 0000000000000ff8 GPR20: 0000000000001fff 0000000000000007 0000000000000080 00007fff89d90158 GPR24: 0000000002000000 0000000002000000 0000000000000255 0000000000000300 GPR28: c000000001270000 0000000042000000 0000000048ab0778 c000000080647e80 NIP [c0000000000098b8] decrementer_common_virt+0xb8/0x230 LR [c00000000006c0c8] ___do_page_fault+0x3f8/0xb10 --- interrupt: 700 Instruction dump: XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX ---[ end trace 6d28218e0cc3c949 ]--- After: ------------[ cut here ]------------ kernel BUG at arch/powerpc/kernel/exceptions-64s.S:491! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV Modules linked in: CPU: 0 PID: 88 Comm: login Not tainted 5.15.0-rc2-00034-ge057cdade6e5-dirty #2638 NIP: c0000000000098b8 LR: c00000000006bf04 CTR: c0000000000097f0 REGS: c0000000fffb3d60 TRAP: 0700 Not tainted MSR: 9000000000021031 <SF,HV,ME,IR,DR,LE> CR: 24482227 XER: 00040000 CFAR: c0000000000098b0 IRQMASK: 0 GPR00: c00000000006bf04 0000000000002400 c000000001513800 c000000001271868 GPR04: 00000000100f0d29 0000000042000000 0000000000000007 0000000000000009 GPR08: 00000000100f0d29 0000000024482227 0000000000002710 c000000000181b3c GPR12: 9000000000009033 c0000000016b0000 00000000100f0d29 c000000005b22f00 GPR16: 00000000ffff0000 0000000000000001 0000000000000009 00000000100eed90 GPR20: 00000000100eed90 0000000010000000 000000001000a49c 00000000100f1430 GPR24: c000000001271868 0000000002000000 0000000000000215 0000000000000300 GPR28: c000000001271800 0000000042000000 00000000100f0d29 c000000080647860 NIP [c0000000000098b8] decrementer_common_virt+0xb8/0x230 LR [c00000000006bf04] ___do_page_fault+0x234/0xb10 Call Trace: Instruction dump: 4182000c 39400001 48000008 894d0932 714a0001 39400008 408225fc 718a4000 7c2a0b78 3821fcf0 41c20008 e82d0910 <0981fcf0> f92101a0 f9610170 f9810178 ---[ end trace a5dbd1f5ea4ccc51 ]--- Fixes: 0a882e2 ("powerpc/64s/exception: remove bad stack branch") Signed-off-by: Nicholas Piggin <[email protected]> Signed-off-by: Michael Ellerman <[email protected]> Link: https://lore.kernel.org/r/[email protected]

[ Upstream commit 3e607dc ] Emergency stack path was jumping into a 3: label inside the __GEN_COMMON_BODY macro for the normal path after it had finished, rather than jumping over it. By a small miracle this is the correct place to build up a new interrupt frame with the existing stack pointer, so things basically worked okay with an added weird looking 700 trap frame on top (which had the wrong ->nip so it didn't decode bug messages either). Fix this by avoiding using numeric labels when jumping over non-trivial macros. Before: LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV Modules linked in: CPU: 0 PID: 88 Comm: sh Not tainted 5.15.0-rc2-00034-ge057cdade6e5 #2637 NIP: 7265677368657265 LR: c00000000006c0c8 CTR: c0000000000097f0 REGS: c0000000fffb3a50 TRAP: 0700 Not tainted MSR: 9000000000021031 <SF,HV,ME,IR,DR,LE> CR: 00000700 XER: 20040000 CFAR: c0000000000098b0 IRQMASK: 0 GPR00: c00000000006c964 c0000000fffb3cf0 c000000001513800 0000000000000000 GPR04: 0000000048ab0778 0000000042000000 0000000000000000 0000000000001299 GPR08: 000001e447c718ec 0000000022424282 0000000000002710 c00000000006bee8 GPR12: 9000000000009033 c0000000016b0000 00000000000000b0 0000000000000001 GPR16: 0000000000000000 0000000000000002 0000000000000000 0000000000000ff8 GPR20: 0000000000001fff 0000000000000007 0000000000000080 00007fff89d90158 GPR24: 0000000002000000 0000000002000000 0000000000000255 0000000000000300 GPR28: c000000001270000 0000000042000000 0000000048ab0778 c000000080647e80 NIP [7265677368657265] 0x7265677368657265 LR [c00000000006c0c8] ___do_page_fault+0x3f8/0xb10 Call Trace: [c0000000fffb3cf0] [c00000000000bdac] soft_nmi_common+0x13c/0x1d0 (unreliable) --- interrupt: 700 at decrementer_common_virt+0xb8/0x230 NIP: c0000000000098b8 LR: c00000000006c0c8 CTR: c0000000000097f0 REGS: c0000000fffb3d60 TRAP: 0700 Not tainted MSR: 9000000000021031 <SF,HV,ME,IR,DR,LE> CR: 22424282 XER: 20040000 CFAR: c0000000000098b0 IRQMASK: 0 GPR00: c00000000006c964 0000000000002400 c000000001513800 0000000000000000 GPR04: 0000000048ab0778 0000000042000000 0000000000000000 0000000000001299 GPR08: 000001e447c718ec 0000000022424282 0000000000002710 c00000000006bee8 GPR12: 9000000000009033 c0000000016b0000 00000000000000b0 0000000000000001 GPR16: 0000000000000000 0000000000000002 0000000000000000 0000000000000ff8 GPR20: 0000000000001fff 0000000000000007 0000000000000080 00007fff89d90158 GPR24: 0000000002000000 0000000002000000 0000000000000255 0000000000000300 GPR28: c000000001270000 0000000042000000 0000000048ab0778 c000000080647e80 NIP [c0000000000098b8] decrementer_common_virt+0xb8/0x230 LR [c00000000006c0c8] ___do_page_fault+0x3f8/0xb10 --- interrupt: 700 Instruction dump: XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX ---[ end trace 6d28218e0cc3c949 ]--- After: ------------[ cut here ]------------ kernel BUG at arch/powerpc/kernel/exceptions-64s.S:491! Oops: Exception in kernel mode, sig: 5 [#1] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV Modules linked in: CPU: 0 PID: 88 Comm: login Not tainted 5.15.0-rc2-00034-ge057cdade6e5-dirty #2638 NIP: c0000000000098b8 LR: c00000000006bf04 CTR: c0000000000097f0 REGS: c0000000fffb3d60 TRAP: 0700 Not tainted MSR: 9000000000021031 <SF,HV,ME,IR,DR,LE> CR: 24482227 XER: 00040000 CFAR: c0000000000098b0 IRQMASK: 0 GPR00: c00000000006bf04 0000000000002400 c000000001513800 c000000001271868 GPR04: 00000000100f0d29 0000000042000000 0000000000000007 0000000000000009 GPR08: 00000000100f0d29 0000000024482227 0000000000002710 c000000000181b3c GPR12: 9000000000009033 c0000000016b0000 00000000100f0d29 c000000005b22f00 GPR16: 00000000ffff0000 0000000000000001 0000000000000009 00000000100eed90 GPR20: 00000000100eed90 0000000010000000 000000001000a49c 00000000100f1430 GPR24: c000000001271868 0000000002000000 0000000000000215 0000000000000300 GPR28: c000000001271800 0000000042000000 00000000100f0d29 c000000080647860 NIP [c0000000000098b8] decrementer_common_virt+0xb8/0x230 LR [c00000000006bf04] ___do_page_fault+0x234/0xb10 Call Trace: Instruction dump: 4182000c 39400001 48000008 894d0932 714a0001 39400008 408225fc 718a4000 7c2a0b78 3821fcf0 41c20008 e82d0910 <0981fcf0> f92101a0 f9610170 f9810178 ---[ end trace a5dbd1f5ea4ccc51 ]--- Fixes: 0a882e2 ("powerpc/64s/exception: remove bad stack branch") Signed-off-by: Nicholas Piggin <[email protected]> Signed-off-by: Michael Ellerman <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sasha Levin <[email protected]>

lategoodbye suggested changes Aug 10, 2018

View reviewed changes

popcornmix added a commit to raspberrypi/firmware that referenced this pull request Aug 10, 2018

kernel: Add rpi-poe-fan driver

953a339

See: raspberrypi/linux#2638 firmware: Add POE HAT support

popcornmix added a commit to Hexxeh/rpi-firmware that referenced this pull request Aug 10, 2018

kernel: Add rpi-poe-fan driver

c806ec4

See: raspberrypi/linux#2638 firmware: Add POE HAT support

XECDesign changed the title ~~Add rpi-poe-fan driver~~ POE HAT driver cleanup (WIP) Aug 28, 2018

XECDesign force-pushed the poe-hat branch from fa4429a to f2d35ab Compare August 28, 2018 09:36

XECDesign changed the title ~~POE HAT driver cleanup (WIP)~~ POE HAT driver cleanup Aug 28, 2018

lassebm reviewed Aug 28, 2018

View reviewed changes

XECDesign added 8 commits August 29, 2018 14:56

rpi-poe-fan: Fix undeclared variable in rpi_poe_fan_suspend

29d1730

closes raspberrypi#2665 Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Add SPDX-License-Identifier

cd96cb4

Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Expand PoE acronym in Kconfig help

ac05da2

Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Give clearer error message on of_property_count_u32_elem…

90373c3

…s fail Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Add documentation

542203d

Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Add vendor to of_device_id compatible string.

108dbb8

Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Rename m_data_s struct to fw_data_s

9f6b06c

Signed-off-by: Serge Schneider <[email protected]>

rpi-poe-fan: Fix typos

e4094a6

Signed-off-by: Serge Schneider <[email protected]>

XECDesign force-pushed the poe-hat branch from 19a8dfe to e4094a6 Compare August 29, 2018 13:58

pelwell merged commit 0f937c8 into raspberrypi:rpi-4.14.y Aug 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

POE HAT driver cleanup #2638

POE HAT driver cleanup #2638

XECDesign commented Aug 6, 2018

lategoodbye commented Aug 10, 2018

pelwell commented Aug 10, 2018

lategoodbye Aug 10, 2018

lategoodbye Aug 10, 2018

JamesH65 Aug 10, 2018

lategoodbye Aug 10, 2018 •

edited

Loading

lategoodbye Aug 10, 2018

XECDesign Aug 28, 2018

lategoodbye Aug 28, 2018

lategoodbye Aug 10, 2018

lategoodbye Aug 10, 2018

XECDesign Aug 28, 2018

pelwell Aug 28, 2018

lategoodbye Aug 10, 2018

XECDesign Aug 10, 2018

lategoodbye Aug 10, 2018

XECDesign Aug 10, 2018

lategoodbye Aug 10, 2018

XECDesign commented Aug 28, 2018 •

edited

Loading

lassebm Aug 28, 2018

lassebm Aug 28, 2018

lassebm Aug 28, 2018

XECDesign commented Aug 29, 2018

POE HAT driver cleanup #2638

POE HAT driver cleanup #2638

Conversation

XECDesign commented Aug 6, 2018

lategoodbye commented Aug 10, 2018

pelwell commented Aug 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lategoodbye Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XECDesign commented Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XECDesign commented Aug 29, 2018

lategoodbye Aug 10, 2018 •

edited

Loading

XECDesign commented Aug 28, 2018 •

edited

Loading