mbox series

[v3,0/8] PMBus fixes and new functions

Message ID 20231023180837.91785-1-titusr@google.com (mailing list archive)
Headers show
Series PMBus fixes and new functions | expand

Message

Titus Rwantare Oct. 23, 2023, 6:08 p.m. UTC
This patch series contains fixes and improvements to PMBus support in QEMU.

The following has been added:
   - Support for block receive
   - Support for devices with fans
   - Support for the VCAP register for devices with onboard energy storage
   - A bitfield struct for the vout mode register, whose bits determine the formatting of several read commands in PMBus
Fixes:
   - String read now handles now logs an error when passed an empty string

This series is in preparation for some additional sensors that exercise
this functionality that will be incoming shortly.

Thanks

Changes in v3:
   - Added fixes to PMBus: page resets and fault clearing

Changes in v2:
   - Expanded commit descriptions
   - Added the ADM1266 device model that uses new functions

Titus Rwantare (8):
  hw/i2c: pmbus add support for block receive
  hw/i2c: pmbus: add vout mode bitfields
  hw/i2c: pmbus: add fan support
  hw/i2c: pmbus: add VCAP register
  hw/sensor: add ADM1266 device model
  tests/qtest: add tests for ADM1266
  hw/i2c: pmbus: immediately clear faults on request
  hw/i2c: pmbus: reset page register for out of range reads

 hw/arm/Kconfig                |   1 +
 hw/i2c/pmbus_device.c         | 237 +++++++++++++++++++++++++++++--
 hw/sensor/Kconfig             |   5 +
 hw/sensor/adm1266.c           | 254 ++++++++++++++++++++++++++++++++++
 hw/sensor/meson.build         |   1 +
 include/hw/i2c/pmbus_device.h |  17 +++
 tests/qtest/adm1266-test.c    | 123 ++++++++++++++++
 tests/qtest/max34451-test.c   |  24 ++++
 tests/qtest/meson.build       |   1 +
 9 files changed, 653 insertions(+), 10 deletions(-)
 create mode 100644 hw/sensor/adm1266.c
 create mode 100644 tests/qtest/adm1266-test.c

--
2.42.0.758.gaed0368e0e-goog

Comments

Alex Bennée Oct. 23, 2023, 7:10 p.m. UTC | #1
Titus Rwantare <titusr@google.com> writes:

> This patch series contains fixes and improvements to PMBus support in QEMU.
>
> The following has been added:
>    - Support for block receive
>    - Support for devices with fans
>    - Support for the VCAP register for devices with onboard energy storage
>    - A bitfield struct for the vout mode register, whose bits determine the formatting of several read commands in PMBus
> Fixes:
>    - String read now handles now logs an error when passed an empty string
>
> This series is in preparation for some additional sensors that exercise
> this functionality that will be incoming shortly.

You seem to have missed a number of tags from previous postings:

  https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review

(although I notice we only mention Reviewed-by in the docs)

You can use a tool like b4 to apply a series and collect the tags. It
will also collect the Message-Id's which are also useful.

Once you've fixed up your tags I think as the maintainer you can submit
a pull request.

>
> Thanks
>
> Changes in v3:
>    - Added fixes to PMBus: page resets and fault clearing
>
> Changes in v2:
>    - Expanded commit descriptions
>    - Added the ADM1266 device model that uses new functions
>
> Titus Rwantare (8):
>   hw/i2c: pmbus add support for block receive
>   hw/i2c: pmbus: add vout mode bitfields
>   hw/i2c: pmbus: add fan support
>   hw/i2c: pmbus: add VCAP register
>   hw/sensor: add ADM1266 device model
>   tests/qtest: add tests for ADM1266
>   hw/i2c: pmbus: immediately clear faults on request
>   hw/i2c: pmbus: reset page register for out of range reads
>
>  hw/arm/Kconfig                |   1 +
>  hw/i2c/pmbus_device.c         | 237 +++++++++++++++++++++++++++++--
>  hw/sensor/Kconfig             |   5 +
>  hw/sensor/adm1266.c           | 254 ++++++++++++++++++++++++++++++++++
>  hw/sensor/meson.build         |   1 +
>  include/hw/i2c/pmbus_device.h |  17 +++
>  tests/qtest/adm1266-test.c    | 123 ++++++++++++++++
>  tests/qtest/max34451-test.c   |  24 ++++
>  tests/qtest/meson.build       |   1 +
>  9 files changed, 653 insertions(+), 10 deletions(-)
>  create mode 100644 hw/sensor/adm1266.c
>  create mode 100644 tests/qtest/adm1266-test.c
Titus Rwantare Oct. 23, 2023, 11:53 p.m. UTC | #2
On Mon, 23 Oct 2023 at 12:16, Alex Bennée <alex.bennee@linaro.org> wrote:

> You seem to have missed a number of tags from previous postings:
>
>   https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
>
> (although I notice we only mention Reviewed-by in the docs)
>
> You can use a tool like b4 to apply a series and collect the tags. It
> will also collect the Message-Id's which are also useful.
>
> Once you've fixed up your tags I think as the maintainer you can submit
> a pull request.
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro

Thanks for the tip about b4, I spent some time getting to grips with
it and it's a huge timesaver.
I haven't quite figured out how to get it to produce and send a signed
pull request, but I don't need that just yet.

-Titus
Alex Bennée Oct. 24, 2023, 10:06 a.m. UTC | #3
Titus Rwantare <titusr@google.com> writes:

> On Mon, 23 Oct 2023 at 12:16, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>> You seem to have missed a number of tags from previous postings:
>>
>>   https://qemu.readthedocs.io/en/master/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review
>>
>> (although I notice we only mention Reviewed-by in the docs)
>>
>> You can use a tool like b4 to apply a series and collect the tags. It
>> will also collect the Message-Id's which are also useful.
>>
>> Once you've fixed up your tags I think as the maintainer you can submit
>> a pull request.
>>
>> --
>> Alex Bennée
>> Virtualisation Tech Lead @ Linaro
>
> Thanks for the tip about b4, I spent some time getting to grips with
> it and it's a huge timesaver.
> I haven't quite figured out how to get it to produce and send a signed
> pull request, but I don't need that just yet.

A pull request is really just a GPG signed tag that you push to a repo.
You can use the existing git tooling to create the cover letter for it.

I've included my exact steps at the end of the email but really it comes
down to:

  git tag --sign your-pr-tag
  git push your-pr-tag
  git format-patch <series details>
  git request-pull origin/master your_repo_details your-pr-tag

and finally

  git send-email

My personal exact steps are integrated with my editor but are:

1 Create a branch (optional)
════════════════════════════

  ┌────
  │ git fetch origin
  │ branch="pr/$(date +%d%m%y)-${series}-${version}"
  │ if test -z "$prefix" ; then
  │     git checkout -b $branch origin/master
  │ else
  │     git checkout -b $branch HEAD
  │ fi
  └────


2 Apply patches from mailing list
═════════════════════════════════

  ┌────
  │ if test -z $prefix; then
  │     mkdir -p "${series}.patches"
  │     b4 am -S -t -o "${series}.patches" "${id}"
  │ else
  │     echo "Built tree by hand"
  │ fi
  └────

  ┌────
  │ if test -z $prefix; then
  │     git am ${series}.patches/*.mbx
  │     rm -rf ${series}.patches
  │ else
  │     git log --oneline origin/master..
  │ fi
  └────


3 Check if we are missing any review/comments or tags
═════════════════════════════════════════════════════


4 Check for unreviewed patches
══════════════════════════════


5 Check all sign-offs are there
═══════════════════════════════


6 Check we have only 1 Message-Id per commit
════════════════════════════════════════════


7 Check commits are good
════════════════════════

  We need to ensure we have added our signoff and there is no — ephemera
  left from commit history.

  ┌────
  │ errors=0
  │ commits=0
  │ while read rev; do
  │     author=$(git show -s --format='%an <%ae>' $rev)
  │     body=$(git show -s --format='%B' $rev)
  │     title=$(git show -s --format='%s' $rev)
  │ 
  │     # Check for Author signoff
  │     if ! grep -q "^Signed-off-by: $author" <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"missing author signoff - %h - %an: %s" $rev)
  │     fi
  │ 
  │     # Check for my signoff (fix for quotes)
  │     if ! grep -q "^Signed-off-by: $signoff" <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"missing my signoff - %h - %an: %s" $rev)
  │     fi
  │ 
  │     if ! grep -q "^Message-Id: " <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"missing message id - %h - %an: %s" $rev)
  │     fi
  │ 
  │     # check for unreviewed patches for patches I authored
  │     if [ "$author" = "$signoff" ]; then
  │         if ! grep -q "^Reviewed-by:" <<< "$body"; then
  │             echo $(git log -1 --pretty=format:"unreviewed - %h - %an: %s" $rev)
  │         fi
  │     fi
  │ 
  │     # Check for stray Based-on tags
  │     if grep -q "^Based-on: " <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"has based-on tag - %h - %an: %s" $rev)
  │     fi
  │ 
  │     # Check for stray history
  │     if grep -q "^--" <<< "$body"; then
  │         errors=$((errors+1))
  │         echo $(git log -1 --pretty=format:"has commit history - %h - %an: %s" $rev)
  │     fi
  │ 
  │     commits=$((commits+1))
  │ done < <(git rev-list "origin/master..HEAD")
  │ 
  │ echo "Found $errors errors over $commits commits"
  └────


8 Preparing a QEMU Pull Request
═══════════════════════════════

  We have two properties here, tversion for the tag and pversion for the
  iteration of the PULL. This is to account for re-rolls where we detect
  and issue after tagging but before we send the PR itself.

  ┌────
  │ (let ((tag (format
  │             "pull-%s-%s-%d"
  │             series
  │             (format-time-string "%d%m%y")
  │             tversion)))
  │   (magit-tag-create tag "HEAD" "--sign")
  │   tag)
  └────

  ┌────
  │ set -e
  │ tag=$(git describe)
  │ git push github $tag
  │ if test -z $prefix; then
  │     git push-ci-now gitlab $tag
  │ else
  │     git push-ci gitlab $tag
  │ fi
  │ echo "pushed $tag"
  └────

  ┌────
  │ (if (= 1 pversion)
  │     "PULL"
  │   (format "PULL v%d" pversion))
  └────

  ┌────
  │ if [ -d "${series}.pull" ]; then
  │   rm -rf ${series}.pull
  │ fi
  │ git format-patch --subject-prefix="$subjprefix" --cover-letter origin/master..HEAD -p -o ${series}.pull
  └────

  You can use the $pull macro to fill in the details


9 And send the pull request
═══════════════════════════

  Using the prefix will limit the send to just the cover letter, useful
  for v2+ pull requests

  ┌────
  │ if test -z "$prefix" ; then
  │   git send-email --confirm=never --dry-run --quiet ${mailto} ${series}.pull/*
  │ else
  │   git send-email --confirm=never --dry-run --quiet ${mailto} ${series}.pull/0000*
  │ fi
  └────

  ┌────
  │ if test -z "$prefix" ; then
  │   git send-email --confirm=never --quiet ${mailto} ${series}.pull/*
  │ else
  │   git send-email --confirm=never --quiet ${mailto} ${series}.pull/0000*
  │ fi
  │ rm -rf ${series}.pull
  └────
Philippe Mathieu-Daudé Oct. 24, 2023, 11:50 a.m. UTC | #4
On 24/10/23 12:06, Alex Bennée wrote:

> A pull request is really just a GPG signed tag that you push to a repo.
> You can use the existing git tooling to create the cover letter for it.
> 
> I've included my exact steps at the end of the email but really it comes
> down to:
> 
>    git tag --sign your-pr-tag
>    git push your-pr-tag
>    git format-patch <series details>
>    git request-pull origin/master your_repo_details your-pr-tag
> 
> and finally
> 
>    git send-email
> 
> My personal exact steps are integrated with my editor but are:


> 8 Preparing a QEMU Pull Request
> ═══════════════════════════════

> 9 And send the pull request
> ═══════════════════════════

For these steps I just do:

$ git publish -b origin/master \
     --pull-request --sign-pull --keyid=0xMYKEY

which uses .gitpublish from commit 08bb160e02,
calling get_maintainer.pl for each patch.

Using GSuite, I also have in ~/.gitconfig:

[sendemail]
     smtpServer = smtp.gmail.com
     smtpBatchSize = 1
     smtpReloginDelay = 3
Titus Rwantare Oct. 24, 2023, 4:07 p.m. UTC | #5
On Tue, 24 Oct 2023 at 04:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 24/10/23 12:06, Alex Bennée wrote:
>
> > A pull request is really just a GPG signed tag that you push to a repo.
> > You can use the existing git tooling to create the cover letter for it.
> >
> > I've included my exact steps at the end of the email but really it comes
> > down to:
> >
> >    git tag --sign your-pr-tag
> >    git push your-pr-tag
> >    git format-patch <series details>
> >    git request-pull origin/master your_repo_details your-pr-tag
> >
> > and finally
> >
> >    git send-email
> >
> > My personal exact steps are integrated with my editor but are:
>
>
> > 8 Preparing a QEMU Pull Request
> > ═══════════════════════════════
>
> > 9 And send the pull request
> > ═══════════════════════════
>
> For these steps I just do:
>
> $ git publish -b origin/master \
>      --pull-request --sign-pull --keyid=0xMYKEY
>
> which uses .gitpublish from commit 08bb160e02,
> calling get_maintainer.pl for each patch.
>
> Using GSuite, I also have in ~/.gitconfig:
>
> [sendemail]
>      smtpServer = smtp.gmail.com
>      smtpBatchSize = 1
>      smtpReloginDelay = 3

Thanks all, I'll do some dry runs to walk through these approaches.

-Titus
Philippe Mathieu-Daudé Oct. 25, 2023, 5:56 a.m. UTC | #6
On 24/10/23 18:07, Titus Rwantare wrote:
> On Tue, 24 Oct 2023 at 04:50, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 24/10/23 12:06, Alex Bennée wrote:
>>
>>> A pull request is really just a GPG signed tag that you push to a repo.
>>> You can use the existing git tooling to create the cover letter for it.
>>>
>>> I've included my exact steps at the end of the email but really it comes
>>> down to:
>>>
>>>     git tag --sign your-pr-tag
>>>     git push your-pr-tag
>>>     git format-patch <series details>
>>>     git request-pull origin/master your_repo_details your-pr-tag
>>>
>>> and finally
>>>
>>>     git send-email
>>>
>>> My personal exact steps are integrated with my editor but are:
>>
>>
>>> 8 Preparing a QEMU Pull Request
>>> ═══════════════════════════════
>>
>>> 9 And send the pull request
>>> ═══════════════════════════
>>
>> For these steps I just do:
>>
>> $ git publish -b origin/master \
>>       --pull-request --sign-pull --keyid=0xMYKEY
>>
>> which uses .gitpublish from commit 08bb160e02,
>> calling get_maintainer.pl for each patch.
>>
>> Using GSuite, I also have in ~/.gitconfig:
>>
>> [sendemail]
>>       smtpServer = smtp.gmail.com
>>       smtpBatchSize = 1
>>       smtpReloginDelay = 3
> 
> Thanks all, I'll do some dry runs to walk through these approaches.

Tell me if you want me to unqueue your v4, otherwise I'll send a PR
with it in a few days.

Regards,

Phil.