Conversation
8411d1b to
a17a53a
Compare
01e1cf6 to
c482653
Compare
|
The ltc4283 driver is already upstream! Any reason this device cannot be supported in that driver? |
|
I was pulled late into the project (around May), so I simply continued work until I ended up being the submitter. I wasn't aware of ltc4283 being in the works, and I think we can't easily add on ltc4282. Digging some history there's this #3126 that seemed to have decided why we have a separate driver code. but now that I see LTC4283 I am inclined to just add support on it, the code overlap seems considerable compared to probable extra code lines. |
b275459 to
5e45c36
Compare
|
v2:
For the gpio support, I was thinking since it's exactly the same feature, just mentioning it in kconfig was enough |
5e45c36 to
dc0ff7e
Compare
ec684c3 to
16e3a40
Compare
c70491f to
666afcf
Compare
8e6ac06 to
f44a9bf
Compare
The LTC4283 is a negative voltage hot swap controller that drives an external N-channel MOSFET to allow a board to be safely inserted and removed from a live backplane. Note that this device has a MODE pin that is determined by differing voltage levels (VEE, VIN, INTVCC, or left open) and cannot be controlled via software or GPIO. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Support the LTC4284 Hot Swap Controller. It is very Similar to LTC4283 but for higher power. The device features programmable current limit with foldback and independently adjustable inrush current to optimize the MOSFET safe operating area (SOA). The SOA timer limits MOSFET temperature rise for reliable protection against overstresses. An I2C interface and onboard ADC allow monitoring of board current, voltage, power, energy, and fault status. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
Similar to LTC4283, LTC4284 has up to 8 pins that can be configured as GPIOs and behaves the same. Signed-off-by: Alexis Czezar Torreno <alexisczezar.torreno@analog.com>
f44a9bf to
6deff73
Compare
|
PR got stale, just following up on review @nunojsa |
a7c82e2 to
a249931
Compare
nunojsa
left a comment
There was a problem hiding this comment.
I think the major thing to rethink is sense1 and sense2. I think it should just be a new sense2 variable! The docs are actually wrong IMO as it looks like we can ever have curr1, curr2 and curr3 which is not the case!
| description: | ||
| Value of the SENSE2 current sense resistor. Only applicable to LTC4284. | ||
| This resistor is connected between SENSE2+ and SENSE2- pins and is used | ||
| for channel 2 current measurements. |
There was a problem hiding this comment.
We should not need those "only applicable to ltc4284". The below conditional stuff should make it clear!
There was a problem hiding this comment.
I see, will lessen comments like this.
| then: | ||
| required: | ||
| - adi,rsense1-nano-ohms | ||
| - adi,rsense2-nano-ohms |
There was a problem hiding this comment.
Would be nice to also add an example for the new chip
| For LTC4284, this is optional and represents the sense resistor value for | ||
| current measurements via the ADC+/ADC- pins (channel 0), which depends on | ||
| the hardware connection (can be wired to SENSE1, SENSE2, or an average via | ||
| resistive divider). |
There was a problem hiding this comment.
I would do the above change
There was a problem hiding this comment.
sorry, can I get further explanation on this?
| the hardware connection (can be wired to SENSE1, SENSE2, or an average via | ||
| resistive divider). | ||
|
|
||
| adi,rsense1-nano-ohms: |
There was a problem hiding this comment.
maybe we can reuse adi,rsense-nano-ohms to be the first one in here?!
| curr3_min_alarm Undercurrent alarm | ||
| curr3_max_alarm Overcurrent alarm | ||
| curr3_crit_alarm Critical Overcurrent alarm | ||
| curr3_label Channel label (ISENSE2) |
There was a problem hiding this comment.
Why 2 and 3? I would say we just have 1 and 2 no?
There was a problem hiding this comment.
Was running on the assumption SENSE, SENSE1, and SENSE2 are all different, as commented below I shall double check this
|
|
||
| config SENSORS_LTC4283 | ||
| tristate "Analog Devices LTC4283" | ||
| tristate "Analog Devices LTC4283 and compatibles" |
| #define LTC4283_POWER_MIN 0x48 | ||
| #define LTC4283_POWER_MAX 0x49 | ||
| #define LTC4283_RESERVED_68 0x68 | ||
| #define LTC4283_RESERVED_6D 0x6D |
There was a problem hiding this comment.
Why have you removed the above? Aren't they still reserved for ltc4283?
There was a problem hiding this comment.
I think I was thinking this was a placeholder especially since ltc4284 uses these, but well, yeah... it doesn't make sense on the view of ltc4283, will revert.
| /* in tenths of microohm - For LTC4284 channel 1 (SENSE1) */ | ||
| u32 rsense1; | ||
| /* in tenths of microohm - For LTC4284 channel 2 (SENSE2) */ | ||
| u32 rsense2; |
There was a problem hiding this comment.
Again! For the registers defintions, sure they are different. But in here I don't see why we need two new rsense1 and 2. rsense can be repurposed to be the "rsense1" no?
There was a problem hiding this comment.
will try to repurpose 'rsense', but yes it should be possible
| }; | ||
|
|
||
| struct ltc4283_chip_info { | ||
| enum ltc4283_chip_id id; |
There was a problem hiding this comment.
I think that instead of an id, what we might need is has_sense2 boolean?!
There was a problem hiding this comment.
will check out this out!
b328ade to
e943ef6
Compare
yes, I may need to clarify this with apps, double check sense, sense1, sense2. would answer a lot of review points above. |
|
apps replied, SENSE is just the average of SENSE1 and SENSE2 for ltc4284. So ltc4283 = SENSE. Will not support the average as it's not an actual real channel |
e943ef6 to
935f1d8
Compare
PR Description
The LTC4284 is a high-power hot swap controller designed for -48V distributed power systems supporting up to 2500W applications. It features dual-gate MOSFET drivers with multiple operation modes, comprehensive monitoring capabilities, and advanced protection.
Datasheet: LTC4284
This PR is for upstreaming
This is for dev branch in case some tests/features are requested: #3351
This supersedes the old draft PR #3126
Key features:
PR Type
PR Checklist