Page MenuHomeFreeBSD
Differential D50870

regulator: don't use internal gpiobus function
ClosedPublic
Actions

Authored by vexeduxr on Jun 16 2025, 7:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Jul 6, 12:23 PM
Unknown Object (File)
Sat, Jul 5, 11:34 AM
Unknown Object (File)
Sat, Jul 5, 2:45 AM
Unknown Object (File)
Fri, Jul 4, 10:40 PM
Unknown Object (File)
Fri, Jul 4, 1:35 AM
Unknown Object (File)
Mon, Jun 30, 3:42 PM
Unknown Object (File)
Sun, Jun 29, 6:13 PM
Unknown Object (File)
Fri, Jun 27, 2:01 AM
View All 16 Files
Subscribers
loos

Details

Reviewers
imp
wulf
manu
mmel
Commits
rG7f297cdae380: regulator: don't use internal gpiobus function
Summary

gpiobus_acquire_pin is only meant to be used internally by gpiobus. Use
gpio_pin_acquire instead.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

vexeduxr created this revision.Jun 16 2025, 7:51 AM
Herald added a subscriber: loos. · View Herald TranscriptJun 16 2025, 7:51 AM
vexeduxr requested review of this revision.Jun 16 2025, 7:51 AM
Harbormaster completed remote builds in B64897: Diff 157091.Jun 16 2025, 7:51 AM
vexeduxr added a child revision: D50871: gpiopps: don't use internal gpiobus function.Jun 16 2025, 7:51 AM
vexeduxr added a parent revision: D50869: gpiobus: add gpio_pin_acquire.Jun 16 2025, 7:53 AM
vexeduxr added a reviewer: manu.
manu accepted this revision.Tue, Jun 17, 7:58 AM
manu added a reviewer: mmel.

I think that it should be ok but let's wait for @mmel

This revision is now accepted and ready to land.Tue, Jun 17, 7:59 AM
mmel added a comment.Tue, Jun 17, 1:25 PM

Why did you remove the comment? IMHO it is still a bit problematic (or not well defined) to hold the mutex across the GPIO infrastructure call.

vexeduxr added a comment.Tue, Jun 17, 5:00 PM

I don't really see why it would cause problems. With the current implementation at least.
Thinking about this some more, it's very possible that a future change to gpiobus_acquire_pin or gpio_pin_acquire can break this though...

vexeduxr updated this revision to Diff 157203.Tue, Jun 17, 5:09 PM

re-add comment.

This revision now requires review to proceed.Tue, Jun 17, 5:09 PM
Harbormaster completed remote builds in B64946: Diff 157203.Tue, Jun 17, 5:09 PM
mmel accepted this revision.Tue, Jun 17, 6:21 PM

Thanks.
The most problematic variant is GPIO over I2C bus (or other serial bus), where sleepable(SX) locks are required for basic operation. So I just was to be very careful and warn others of the problematic place.

This revision is now accepted and ready to land.Tue, Jun 17, 6:21 PM
imp accepted this revision.Thu, Jul 3, 7:59 PM

Looks good, but this is more 'permission to commit' rather than 'the change is good'. I trust the people that reviewed it and didn't see anything insane.

Closed by commit rG7f297cdae380: regulator: don't use internal gpiobus function (authored by vexeduxr). · Explain WhyFri, Jul 4, 8:26 PM
This revision was automatically updated to reflect the committed changes.
vexeduxr added a commit: rG7f297cdae380: regulator: don't use internal gpiobus function.

Revision Contents
Changeset List

PathSize
sys/
dev/
regulator/
8 lines

Diff 157987

View Options

sys/dev/regulator/regulator_fixed.c

Loading...