All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Jon Derrick <jonathan.derrick@intel.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	linux-pci@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Andy Shevchenko <andriy.shevchenko@intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Pawel Baldysiak <pawel.baldysiak@intel.com>,
	Sinan Kaya <okaya@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Keith Busch <kbusch@kernel.org>,
	Alexandru Gagniuc <mr.nuke.me@gmail.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [RFC 1/9] PCI: pci-bridge-emul: Update PCIe register behaviors
Date: Sat, 8 Feb 2020 11:55:43 +0200	[thread overview]
Message-ID: <CAHp75VeuzA+y2RzDiirskuZmwiBX4J3xz4S6jd_bikScQ52sRw@mail.gmail.com> (raw)
In-Reply-To: <1581120007-5280-2-git-send-email-jonathan.derrick@intel.com>

On Sat, Feb 8, 2020 at 2:02 AM Jon Derrick <jonathan.derrick@intel.com> wrote:
>
> Update the PCIe register behaviors and comments for PCIe v5.0.

I think you may elaborate the changes here, like a summary.

> Replace the specific bit definitions with BIT and GENMASK to make
> updating easier in the future.

...

> +                * Device status register has bits 6 and [3:0] W1C, [5:4] RO,
> +                * the rest is reserved

Perhaps it makes sense to add '.' (period) to the end of sentence. And
the same for the rest comments.

...

> -               .w1c = (PCI_EXP_SLTSTA_ABP | PCI_EXP_SLTSTA_PFD |
> -                       PCI_EXP_SLTSTA_MRLSC | PCI_EXP_SLTSTA_PDC |
> -                       PCI_EXP_SLTSTA_CC | PCI_EXP_SLTSTA_DLLSC) << 16,
> -               .ro = (PCI_EXP_SLTSTA_MRLSS | PCI_EXP_SLTSTA_PDS |
> -                      PCI_EXP_SLTSTA_EIS) << 16,

> +               .w1c = (BIT(8) | GENMASK(4, 0)) << 16,
> +               .ro = GENMASK(7, 5) << 16,

I'm not sure the new one is better. Perhaps you need to add
description for the new bits and, if you consider it's needed, add a
picture of the bits in the comment, like  XXXX rwX1 XXrX XwXX, where r
= ro, w = rw, 1 = w1c, X = rsvd. But it's up to Bjorn and you, I have
no strong opinion here.

Same for the rest similar changes.

...

> -               .rw = (PCI_EXP_RTCTL_SECEE | PCI_EXP_RTCTL_SENFEE |
> -                      PCI_EXP_RTCTL_SEFEE | PCI_EXP_RTCTL_PMEIE |
> -                      PCI_EXP_RTCTL_CRSSVE),
> -               .ro = PCI_EXP_RTCAP_CRSVIS << 16,
> +               .rw = GENMASK(4, 0),
> +               .ro = BIT(0) << 16,

Bit 0 in both rw and ro -- is it correct?

-- 
With Best Regards,
Andy Shevchenko

  reply	other threads:[~2020-02-08  9:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 23:59 [RFC 0/9] PCIe Hotplug Slot Emulation driver Jon Derrick
2020-02-07 23:59 ` [RFC 1/9] PCI: pci-bridge-emul: Update PCIe register behaviors Jon Derrick
2020-02-08  9:55   ` Andy Shevchenko [this message]
2020-03-28 21:42   ` Bjorn Helgaas
2020-02-08  0:00 ` [RFC 2/9] PCI: pci-bridge-emul: Eliminate reserved member Jon Derrick
2020-03-28 21:43   ` Bjorn Helgaas
2020-02-08  0:00 ` [RFC 3/9] PCI: pci-bridge-emul: Provide a helper to set behavior Jon Derrick
2020-02-08  0:00 ` [RFC 4/9] PCI: pciehp: Indirect slot register operations Jon Derrick
2020-02-08  0:00 ` [RFC 5/9] PCI: Add pcie_port_slot_emulated stub Jon Derrick
2020-02-08  0:00 ` [RFC 6/9] PCI: pciehp: Expose the poll loop to other drivers Jon Derrick
2020-02-08  0:00 ` [RFC 7/9] PCI: Move pci_dev_str_match to search.c Jon Derrick
2020-02-08  0:00 ` [RFC 8/9] PCI: pciehp: Add hotplug slot emulation driver Jon Derrick
2020-02-08  0:00 ` [RFC 9/9] PCI: pciehp: Wire up pcie_port_emulate_slot and pciehp_emul Jon Derrick
2020-02-10  7:01 ` [RFC 0/9] PCIe Hotplug Slot Emulation driver Christoph Hellwig
2020-02-10 15:05   ` Derrick, Jonathan
2020-02-10 16:58     ` hch
2020-02-10 17:09       ` Derrick, Jonathan
2020-03-28 21:51 ` Bjorn Helgaas
2020-03-30 17:43   ` Derrick, Jonathan
2020-03-30 17:49     ` hch
2020-04-01 21:45     ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHp75VeuzA+y2RzDiirskuZmwiBX4J3xz4S6jd_bikScQ52sRw@mail.gmail.com \
    --to=andy.shevchenko@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=hch@lst.de \
    --cc=helgaas@kernel.org \
    --cc=jonathan.derrick@intel.com \
    --cc=kbusch@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mr.nuke.me@gmail.com \
    --cc=okaya@kernel.org \
    --cc=pawel.baldysiak@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.