Thread (37 messages) 37 messages, 4 authors, 2017-10-01

Re: [PATCH v3 0/5] bq27xxx_battery data memory update

From: Liam Breck <hidden>
Date: 2017-08-29 22:29:56

Hi Andrew,

On Tue, Aug 29, 2017 at 8:31 AM, Andrew F. Davis [off-list ref] wrote:
On 08/29/2017 05:54 AM, Sebastian Reichel wrote:
quoted
Hi,

On Wed, Aug 23, 2017 at 08:36:12PM -0700, Liam Breck wrote:
quoted
Overview:
* Reorganizes chip data definitions
* Enables features landed in these patches:
  dt-bindings: power: supply: bq27xxx: Add monitored-battery documentation
  power: supply: bq27xxx: Add chip data memory read/write support
  power: supply: bq27xxx: Add power_supply_battery_info support
* Supports the following chips (only BQ27425 is active)
  BQ27500, 545, 425, 421, 441, 621

Changes in v3:
* BQ27425 tested; workaround minor chip bug
* Dropped driver_version
* Fixed dbg_dupes logic for .props & .dm_regs
* Dropped two props array dupes

Changes in v2:
* Added di->opts flags for remaining chip features
* Commented out untested bq27xxx_dm_regs parameters
* Changed dbg_dupes to run only once

Notes on v1:
* Not fully tested (hence RFC tag)
Thanks, full series queued.

-- Sebastian
Hold up, I'm just now seeing this series, looks like Liam left me out of
the CC despite being a reviewer in the MAINTAINERS file.. (possibly due
to me actually reviewing these patches and making him fix shit). I've
actively NACK'd some of these changes previously, making this all the
sneakier -_-

Anyway, I've not got the time to fight these changes anymore, but at
very least could you drop 4/5, it's static analysis code made into a
runtime check built into a kernel driver, if not at least add my
nacked-by. :)
I switched to CCing Pali, also a listed maintainer.

In our final discussion of the initial series enabling this feature
(which Sebastian queued without your ack) I found your comments to be
unhelpful, and your followups to my questions and clarifications, if
you provided any, to be inscrutable:

https://patchwork.kernel.org/patch/9710815/
https://patchwork.kernel.org/patch/9710817/
https://patchwork.kernel.org/patch/9710819/

That was after a sequence of 15 revisions (incl partial series) over 3
months, each of which you reviewed casually, forcing me to produce
repeated revs with few changes. Where you commented, your tone was
often pejorative. And you offered to, but never did, test this on
chips I don't have. I sadly concluded that you did not respect my time
nor my perspective. After Sebastian queued the series without your
ack, I felt it was reasonable to drop you from CC.

Sebastian raised two things you had complained about: 1) the real/fake
chip IDs, which he suggested a solution for; and 2) the data dupes
checking, whereon he agreed with me that it could be moved to static
analysis later by someone with knowledge of those tools.

BTW another bq27xxx patchset was submitted on April 30, and you made
no comment on it:

https://patchwork.kernel.org/patch/9706157/
https://patchwork.kernel.org/patch/9706159/
https://patchwork.kernel.org/patch/9706165/
https://patchwork.kernel.org/patch/9706169/
https://patchwork.kernel.org/patch/9706175/

Liam
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help