RE: [PATCH v2 3/3] hpsa: add an assert to prevent from __packed reintroduction

2021-04-02 Thread Elliott, Robert (Servers)
It looks like ia64 implements atomic_t as a 64-bit value and expects atomic_t
to be 64-bit aligned, but does nothing to ensure that.

For x86, atomic_t is a 32-bit value and atomic64_t is a 64-bit value, and
the definition of atomic64_t is overridden in a way that ensures
64-bit (8 byte) alignment:

Generic definitions are in include/linux/types.h:
typedef struct {
int counter;
} atomic_t;

#define ATOMIC_INIT(i) { (i) }

#ifdef CONFIG_64BIT
typedef struct {
s64 counter;
} atomic64_t;
#endif

Override in arch/x86/include/asm/atomic64_32.h:
typedef struct {
s64 __aligned(8) counter;
} atomic64_t;

Perhaps ia64 needs to take over the definition of both atomic_t and atomic64_t
and do the same?



RE: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

2021-03-04 Thread Elliott, Robert (Servers)



> -Original Message-
> From: Brad Larson 
> Sent: Wednesday, March 3, 2021 9:42 PM
> Subject: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control
.
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
...
> +config GPIO_ELBA_SPICS
> + bool "Pensando Elba SPI chip-select"
> + depends on ARCH_PENSANDO_ELBA_SOC
> + help
> +   Say yes here to support the Pensndo Elba SoC SPI chip-select
> driver

Pensndo should be Pensando

> diff --git a/drivers/gpio/gpio-elba-spics.c b/drivers/gpio/gpio-elba-spics.c
> + * Pensando Elba ASIC SPI chip select driver
...
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Elba SPI chip-select driver");

I think it's conventional to include the company name there, so
start that with "Pensando Elba"

Also, "SoC" and "ASIC" are sometimes included after Elba, but sometimes
are not. Consistency might be helpful.

> +static const struct of_device_id ebla_spics_of_match[] = {
...
> + .of_match_table = ebla_spics_of_match,

ebla should be elba




RE: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default

2019-06-18 Thread Elliott, Robert (Servers)


> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org 
> [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of Bart
> Van Assche
> Sent: Monday, June 17, 2019 10:28 PM
> To: dgilb...@interlog.com; Marc Gonzalez ; James 
> Bottomley
> ; Martin Petersen 
> Cc: SCSI ; LKML ; 
> Christoph Hellwig
> 
> Subject: Re: [PATCH v1] scsi: Don't select SCSI_PROC_FS by default
> 
> On 6/17/19 5:35 PM, Douglas Gilbert wrote:
> > For sg3_utils:
> >
> > $ find . -name '*.c' -exec grep "/proc/scsi" {} \; -print
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sg_read.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sgp_dd.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sgm_dd.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./src/sg_dd.c
> >      "'echo 1 > /proc/scsi/sg/allow_dio'\n", q_len,
> > dirio_count);
> > ./testing/sg_tst_bidi.c
> > static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
> > ./examples/sgq_dd.c
> >
> > That is 6 (not 38) by my count.
> 
> Hi Doug,
> 
> This is the command I ran:
> 
> $ git grep /proc/scsi | wc -l
> 38
> 
> I think your query excludes scripts/rescan-scsi-bus.sh.
> 
> Bart.

Here's the full list to ensure the discussion doesn't overlook anything:

sg3_utils-1.44$ grep -R /proc/scsi .
./src/sg_read.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sgp_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sgm_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./src/sg_dd.c:static const char * proc_allow_dio = "/proc/scsi/sg/allow_dio";
./scripts/rescan-scsi-bus.sh:# Return hosts. /proc/scsi/HOSTADAPTER/? must exist
./scripts/rescan-scsi-bus.sh:  for driverdir in /proc/scsi/*; do
./scripts/rescan-scsi-bus.sh:driver=${driverdir#/proc/scsi/}
./scripts/rescan-scsi-bus.sh:  name=${hostdir#/proc/scsi/*/}
./scripts/rescan-scsi-bus.sh:# Get /proc/scsi/scsi info for device 
$host:$channel:$id:$lun
./scripts/rescan-scsi-bus.sh:SCSISTR=$(grep -A "$LN" -e "$grepstr" 
/proc/scsi/scsi)
./scripts/rescan-scsi-bus.sh:DRV=`grep 'Attached drivers:' /proc/scsi/scsi 
2>/dev/null`
./scripts/rescan-scsi-bus.sh:  echo "scsi report-devs 1" >/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:  DRV=`grep 'Attached drivers:' 
/proc/scsi/scsi 2>/dev/null`
./scripts/rescan-scsi-bus.sh:  echo "scsi report-devs 0" >/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:# Outputs description from /proc/scsi/scsi (unless 
arg passed)
./scripts/rescan-scsi-bus.sh:echo "scsi remove-single-device $devnr" > 
/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:  echo "scsi add-single-device $devnr" > 
/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:  echo "scsi add-single-device $devnr" > 
/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:  echo "scsi add-single-device $devnr" > 
/proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:  echo "scsi add-single-device $host $channel 
$id $SCAN_WILD_CARD" > /proc/scsi/scsi
./scripts/rescan-scsi-bus.sh:if test ! -d /sys/class/scsi_host/ -a ! -d 
/proc/scsi/; then
./ChangeLog:/proc/scsi/sg/allow_dio is '0'
./ChangeLog:  - change sg_debug to call system("cat /proc/scsi/sg/debug");
./suse/sg3_utils.changes:  * Support systems without /proc/scsi
./examples/sgq_dd.c:static const char * proc_allow_dio = 
"/proc/scsi/sg/allow_dio";
./doc/sg_read.8:If direct IO is selected and /proc/scsi/sg/allow_dio
./doc/sg_read.8:"echo 1 > /proc/scsi/sg/allow_dio". An alternate way to avoid 
the
./doc/sg_map.8:observing the output of the command: "cat /proc/scsi/scsi".
./doc/sgp_dd.8:at completion. If direct IO is selected and 
/proc/scsi/sg/allow_dio
./doc/sgp_dd.8:this at completion. If direct IO is selected and 
/proc/scsi/sg/allow_dio
./doc/sgp_dd.8:mapping to SCSI block devices should be checked with 'cat 
/proc/scsi/scsi'
./doc/sg_dd.8:notes this at completion. If direct IO is selected and 
/proc/scsi/sg/allow_dio
./doc/sg_dd.8:this at completion. If direct IO is selected and 
/proc/scsi/sg/allow_dio
./doc/sg_dd.8:with 'echo 1 > /proc/scsi/sg/allow_dio'.
./doc/sg_dd.8:mapping to SCSI block devices should be checked with 'cat 
/proc/scsi/scsi',




RE: [PATCH v2] checkpatch: add command-line option for TAB size

2019-05-08 Thread Elliott, Robert (Servers)


> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org 
> [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of
> Antonio Borneo
> Sent: Wednesday, May 8, 2019 12:44 PM
> Subject: [PATCH v2] checkpatch: add command-line option for TAB size
...
> Add a command-line option "--tab-size" to let the user select a
> TAB size value other than 8.
> This makes easy to reuse this script by other projects with
> different requirements in their coding style (e.g. OpenOCD [1]
> requires TAB size of 4 characters [2]).
...
> +  --tab-size=n   set the number of spaces for tab (default 8)
...
> + 'tab-size=i'=> \$tabsize,
...
> - for (; ($n % 8) != 0; $n++) {
> + for (; ($n % $tabsize) != 0; $n++) {
...
> - if ($indent % 8) {
> + if ($indent % $tabsize) {
> - "\t" x ($pos / 8) .
> - " "  x ($pos % 8);
> + "\t" x ($pos / $tabsize) .
> + " "  x ($pos % $tabsize);
...
> - (($sindent % 8) != 0 ||
> + (($sindent % $tabsize) != 0 ||
...
> -  ($sindent > $indent + 8))) {
> +  ($sindent > $indent + $tabsize))) {

Checking for 0 before using the value in division and modulo
operations would be prudent.



RE: [PATCH v2 3/5] PCI/ATS: Skip VF ATS initialization if PF does not implement it

2019-05-06 Thread Elliott, Robert (Servers)



> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org 
> [mailto:linux-kernel-ow...@vger.kernel.org] On Behalf Of
> sathyanarayanan.kuppusw...@linux.intel.com
> Sent: Monday, May 6, 2019 12:20 PM
> To: bhelg...@google.com
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; 
> ashok@intel.com;
> keith.bu...@intel.com; sathyanarayanan.kuppusw...@linux.intel.com
> Subject: [PATCH v2 3/5] PCI/ATS: Skip VF ATS initialization if PF does not 
> implement it
> 
> From: Kuppuswamy Sathyanarayanan 
> 
> If PF does not implement ATS and VF implements/uses it, it might lead to
> runtime issues. Also, as per spec r4.0, sec 9.3.7.8, PF should implement
> ATS if VF implements it. So add additional check to confirm given device
> aligns with the spec.
...
> + /*
> +  * Per PCIe r4.0, sec 9.3.7.8, if VF implements Address Translation
> +  * Services (ATS) Extended Capability then corresponding PF should
> +  * also implement it.
> +  */
...

In standardese, "should" means recommended, not required. The PCIe spec uses
"must" for this rule; the comments should match.