cpu/sam0: enable PD for MISO pin to save energy

[haukepetersen]

Factored out from #6652 to get this merged quickly (credits go to @immesys). Seems like I lost this (previously already merged change: #5868) when doing the SPI remodeling. Sorry for that...

[jnohlgard]

Is it not better to switch the pin to high impedance (HiZ, probably analog function in the mux) than to add a pull resistor? Some slave devices also have pull resistors, which may conflict with this and create a direct resistance path between vdd and ground.
Is pull down the correct choice for the idle state of the bus?

[haukepetersen]

Is it not better to switch the pin to high impedance

I think this won't work, as this pin configuration is used while the bus is used actively also, and in this case configuring it to AIN would break the SPI functionality?!

Is pull down the correct choice for the idle state of the bus?

Actually, I am not aware of a 'correct' bus state. It seems to me, it depends on the board layout and connected devices. Now thinking of it, the 'correct' solution for us would probably to make the MISO pin mode configurable through the boards periph_conf?!

[jnohlgard]

I think you misunderstood, I meant, switch the mux to analog between each transmission because there's nothing driving the bus if the master (we) does not assert any CS pins.

[immesys]

For correct bus state, if no CS pin is asserted, the master owns MISO and can choose whatever state it wants on it. If there are slaves with pulldowns or pullups on MISO that are active when CS is deasserted then they are at fault.

[immesys]

Also, I know this was factored out of #6652 because it is an uncontroversial change that was already previously merged in #5868, but the other changes I mention in #6652 are also critical.

I don't mind if there are philosphical disagreements about what the correct thing to do is, but pick anything that solves the SPI power consumption in sleep problem and go with it. Power consumption is one of the biggest metrics of fitness for purpose in a wireless sensor network operating system, and arguably should be tested before each new release. It's disheartening for us to find critical power consumption regressions, push fixes upstream, and then have them treated as low priority and ignored for months.

[haukepetersen]

It's disheartening for us to find critical power consumption regressions, push fixes upstream, and then have them treated as low priority and ignored for months.

Yes indeed. I wouldn't say it's low priority on our side, I'd rather say its one of too many high priority topics... And sorry if there is not enough movement on the low-power side, we should be more pressing on these topics (PM implemenation, CI integration, ...) indeed!

@gebart: would the argument given above be enough to convince you to ACK this PR?

[jnohlgard]

I won't block this PR since there is a serious problem on that platform which this PR fixes

[immesys]

I think we would be willing to put in the effort to help build power monitoring CI into murdock, especially for samr21 as a pilot. We have a lot of that internally. Let's chat offline and I can share some of our experiences and solutions

[thomaseichinger]

@immesys I'd appreciate if you could keep me in the loop regarding CI power monitoring.

[haukepetersen]

@immesys: sounds great. I'll ping you off-list so we can coordinate on a call sometime this week to sync on energy profiling in RIOT's CI if thats ok?!

@everyone: someone willing to ACK this PR so we get back to energy-niceness?

[haukepetersen]

never mind, closing this in favor of #6652