[PATCH 00/29] treewide: Convert comma separated statements

2020-08-24 Thread Joe Perches
There are many comma separated statements in the kernel.
See:https://lore.kernel.org/lkml/alpine.DEB.2.22.394.2008201856110.2524@hadrien/

Convert the comma separated statements that are in if/do/while blocks
to use braces and semicolons.

Many comma separated statements still exist but those are changes for
another day.

Joe Perches (29):
  coding-style.rst: Avoid comma statements
  alpha: Avoid comma separated statements
  ia64: Avoid comma separated statements
  sparc: Avoid comma separated statements
  ata: Avoid comma separated statements
  drbd: Avoid comma separated statements
  lp: Avoid comma separated statements
  dma-buf: Avoid comma separated statements
  drm/gma500: Avoid comma separated statements
  drm/i915: Avoid comma separated statements
  hwmon: (scmi-hwmon): Avoid comma separated statements
  Input: MT - Avoid comma separated statements
  bcache: Avoid comma separated statements
  media: Avoid comma separated statements
  mtd: Avoid comma separated statements
  8390: Avoid comma separated statements
  fs_enet: Avoid comma separated statements
  wan: sbni: Avoid comma separated statements
  s390/tty3270: Avoid comma separated statements
  scai/arm: Avoid comma separated statements
  media: atomisp: Avoid comma separated statements
  video: fbdev: Avoid comma separated statements
  fuse: Avoid comma separated statements
  reiserfs: Avoid comma separated statements
  lib/zlib: Avoid comma separated statements
  lib: zstd: Avoid comma separated statements
  ipv6: fib6: Avoid comma separated statements
  sunrpc: Avoid comma separated statements
  tools: Avoid comma separated statements

 Documentation/process/coding-style.rst|  17 +
 arch/alpha/kernel/pci_iommu.c |   8 +-
 arch/alpha/oprofile/op_model_ev4.c|  22 +-
 arch/alpha/oprofile/op_model_ev5.c|   8 +-
 arch/ia64/kernel/smpboot.c|   7 +-
 arch/sparc/kernel/smp_64.c|   7 +-
 drivers/ata/pata_icside.c |  21 +-
 drivers/block/drbd/drbd_receiver.c|   6 +-
 drivers/char/lp.c |   6 +-
 drivers/dma-buf/st-dma-fence.c|   7 +-
 drivers/gpu/drm/gma500/mdfld_intel_display.c  |  44 ++-
 drivers/gpu/drm/i915/gt/gen8_ppgtt.c  |   8 +-
 drivers/gpu/drm/i915/gt/intel_gt_requests.c   |   6 +-
 .../gpu/drm/i915/gt/selftest_workarounds.c|   6 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c   |   6 +-
 drivers/hwmon/scmi-hwmon.c|   6 +-
 drivers/input/input-mt.c  |  11 +-
 drivers/md/bcache/bset.c  |  12 +-
 drivers/md/bcache/sysfs.c |   6 +-
 drivers/media/i2c/msp3400-kthreads.c  |  12 +-
 drivers/media/pci/bt8xx/bttv-cards.c  |   6 +-
 drivers/media/pci/saa7134/saa7134-video.c |   7 +-
 drivers/mtd/devices/lart.c|  10 +-
 drivers/net/ethernet/8390/axnet_cs.c  |  19 +-
 drivers/net/ethernet/8390/lib8390.c   |  14 +-
 drivers/net/ethernet/8390/pcnet_cs.c  |   6 +-
 .../ethernet/freescale/fs_enet/fs_enet-main.c |  11 +-
 drivers/net/wan/sbni.c| 101 +++---
 drivers/s390/char/tty3270.c   |   6 +-
 drivers/scsi/arm/cumana_2.c   |  19 +-
 drivers/scsi/arm/eesox.c  |   9 +-
 drivers/scsi/arm/powertec.c   |   9 +-
 .../media/atomisp/pci/atomisp_subdev.c|   6 +-
 drivers/video/fbdev/tgafb.c   |  12 +-
 fs/fuse/dir.c |  24 +-
 fs/reiserfs/fix_node.c|  36 ++-
 lib/zlib_deflate/deftree.c|  49 ++-
 lib/zstd/compress.c   | 120 ---
 lib/zstd/fse_compress.c   |  24 +-
 lib/zstd/huf_compress.c   |   6 +-
 net/ipv6/ip6_fib.c|  12 +-
 net/sunrpc/sysctl.c   |   6 +-
 tools/lib/subcmd/help.c   |  10 +-
 tools/power/cpupower/utils/cpufreq-set.c  |  14 +-
 tools/testing/selftests/vm/gup_benchmark.c|  18 +-
 tools/testing/selftests/vm/userfaultfd.c  | 296 +++---
 46 files changed, 694 insertions(+), 382 deletions(-)

-- 
2.26.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 21/29] media: atomisp: Avoid comma separated statements

2020-08-24 Thread Joe Perches
Use semicolons and braces.

Signed-off-by: Joe Perches 
---
 drivers/staging/media/atomisp/pci/atomisp_subdev.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c 
b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 6ba817f15655..52b9fb18c87f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -410,8 +410,10 @@ int atomisp_subdev_set_selection(struct v4l2_subdev *sd,
 
if (atomisp_subdev_format_conversion(isp_sd,
 isp_sd->capture_pad)
-   && crop[pad]->width && crop[pad]->height)
-   crop[pad]->width -= padding_w, crop[pad]->height -= 
padding_h;
+   && crop[pad]->width && crop[pad]->height) {
+   crop[pad]->width -= padding_w;
+   crop[pad]->height -= padding_h;
+   }
 
/* if subdev type is SOC camera,we do not need to set DVS */
if (isp->inputs[isp_sd->input_curr].type == SOC_CAMERA)
-- 
2.26.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Hello

2020-08-24 Thread WIDOW COLON DENISE . G
Hello,

As I thank you for the attention you give to my dearest wish, I 
would like you to know that I did not get the wrong person in 
sending you this message. My ardent wish has always been to meet 
an anonymous natural person so that this last one carries out 
social actions through a foundation. However, I would understand 
your astonishment as to my way of proceeding. My name is COLON 
DENISE .G born July 27, 1942, of French nationality and I am 
currently under medical observation in a hospital in PORTUGAL. I 
had to contact you about this kind because I would like to donate 
a sum of 1,500,000 € to help people in need, to make happy poor 
families, orphans, to help young entrepreneurs who are looking 
for funding for to grow their sectors of activity ... of your 
entourage. My professional life has been a long quiet river, 
especially since I have always lived far from my country. First 
in Kuwait, where I worked in the oil sector for two years. Then I 
went to the Republic of Benin (2001) where I set up several 
companies (real estate, industrial ...). It is in this country so 
welcoming that I knew the real happiness, that of a marriage with 
a Canadian who also worked in this country. Unfortunately we were 
not fortunate enough to have children. After five years of living 
together, my husband lost his life following a long illness. So I 
was left alone again with a staffed butler and a dog until this 
cancer came to limit my life. It will soon be four years that I 
am fighting against this disease and medicine can no longer do 
anything following the results of the medical examinations which 
indicate that my days are numbered, following the confessions of 
my doctor. I had blocked this important sum in one of the Banks 
of Benin for a construction project. I would be grateful to 
entrust you with this money so that my donation project succeeds. 
Please accept this, for it is a gift from a dying woman and that 
without asking for anything in return. Please answer me as 
quickly as possible at my following email address: 
colondeniseg...@outlook.fr

WIDOW COLON DENISE .G
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2] drivers: staging: comedi: fixed duplicate words from checkpatch

2020-08-24 Thread Ethan Edwards
Fixed various different checkpatch duplicate word warnings, the TODO
file said to fix checkpatch warnings.

Signed-off-by: Ethan Edwards 
---
Old email has lines about CC'ing the mailing lists, patch v2 fixes this.
 drivers/staging/comedi/comedi.h  | 4 ++--
 drivers/staging/comedi/comedidev.h   | 2 +-
 drivers/staging/comedi/drivers/addi_apci_1564.c  | 4 ++--
 drivers/staging/comedi/drivers/comedi_8255.c | 2 +-
 drivers/staging/comedi/drivers/ni_tiocmd.c   | 2 +-
 drivers/staging/comedi/drivers/pcmuio.c  | 2 +-
 drivers/staging/comedi/drivers/quatech_daqp_cs.c | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index 09a940066c0e..b5d00a006dbb 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -680,7 +680,7 @@ struct comedi_rangeinfo {
  * value of 1 volt.
  *
  * The only defined flag value is %RF_EXTERNAL (%0x100), indicating that the
- * the range needs to be multiplied by an external reference.
+ * range needs to be multiplied by an external reference.
  */
 struct comedi_krange {
int min;
@@ -970,7 +970,7 @@ enum i8254_mode {
  *   major reasons exist why this caused major confusion for users:
  *   1) The register values are _NOT_ in user documentation, but rather in
  * arcane locations, such as a few register programming manuals that are
- * increasingly hard to find and the NI MHDDK (comments in in example 
code).
+ * increasingly hard to find and the NI MHDDK (comments in example code).
  * There is no one place to find the various valid values of the registers.
  *   2) The register values are _NOT_ completely consistent.  There is no way 
to
  * gain any sense of intuition of which values, or even enums one should 
use
diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 0dff1ac057cd..0e1b95ef9a4d 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -627,7 +627,7 @@ extern const struct comedi_lrange range_unknown;
  * @range: Array of  comedi_krange, one for each range.
  *
  * Each element of @range[] describes the minimum and maximum physical range
- * range and the type of units.  Typically, the type of unit is %UNIT_volt
+ * and the type of units.  Typically, the type of unit is %UNIT_volt
  * (i.e. volts) and the minimum and maximum are in millionths of a volt.
  * There may also be a flag that indicates the minimum and maximum are merely
  * scale factors for an unknown, external reference.
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index fadefcb5c237..06fc7ed96200 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -544,7 +544,7 @@ static int apci1564_timer_insn_write(struct comedi_device 
*dev,
 {
struct apci1564_private *devpriv = dev->private;
 
-   /* just write the last last to the reload register */
+   /* just write the last to the reload register */
if (insn->n) {
unsigned int val = data[insn->n - 1];
 
@@ -628,7 +628,7 @@ static int apci1564_counter_insn_write(struct comedi_device 
*dev,
unsigned int chan = CR_CHAN(insn->chanspec);
unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
 
-   /* just write the last last to the reload register */
+   /* just write the last to the reload register */
if (insn->n) {
unsigned int val = data[insn->n - 1];
 
diff --git a/drivers/staging/comedi/drivers/comedi_8255.c 
b/drivers/staging/comedi/drivers/comedi_8255.c
index 3298725b9ba5..b7ca465933ee 100644
--- a/drivers/staging/comedi/drivers/comedi_8255.c
+++ b/drivers/staging/comedi/drivers/comedi_8255.c
@@ -248,7 +248,7 @@ EXPORT_SYMBOL_GPL(subdev_8255_mm_init);
  * subdev_8255_regbase - get offset of 8255 registers or call-back context
  * @s: comedi subdevice
  *
- * Returns the 'regbase' parameter that was previously passed to to
+ * Returns the 'regbase' parameter that was previously passed to
  * subdev_8255_init() or subdev_8255_mm_init() to set up the subdevice.
  * Only valid if the subdevice was set up successfully.
  */
diff --git a/drivers/staging/comedi/drivers/ni_tiocmd.c 
b/drivers/staging/comedi/drivers/ni_tiocmd.c
index 2a9f7e9821a7..ab6d9e8269f3 100644
--- a/drivers/staging/comedi/drivers/ni_tiocmd.c
+++ b/drivers/staging/comedi/drivers/ni_tiocmd.c
@@ -286,7 +286,7 @@ int ni_tio_cmdtest(struct comedi_device *dev,
 * This should be done, but we don't yet know the actual
 * register values.  These should be tested and then documented
 * in the ni_route_values/ni_*.csv files, with indication of
-* who/when/which/how these these were tested.
+* who/when/which/how these were tested.
 * 

[PATCH v11 00/11] PCI: brcmstb: enable PCIe for STB chips

2020-08-24 Thread Jim Quinlan


Patchset Summary:
  Enhance a PCIe host controller driver.  Because of its unusual design
  we are foced to change dev->dma_pfn_offset into a more general role
  allowing multiple offsets.  See the 'v1' notes below for more info.

v11:
  Commit: "device-mapping: Introduce DMA range map, supplanting ..."
  -- Rebased to latest torvalds, Aug 20, 2020.
  -- Minor change in of_dma_get_range() to satisfy the kernel's
 robot tester.
  -- Use of PFN_DOWN(), PFN_PHYS() instead of explicit shifts (Andy)
  -- Eliminate extra return in dma_offset_from_xxx_addr() (Andy)
  -- Change dma_set_offset_range() to correctly handle the case
 of pre-existing DMA map and zero offset.

v10: 
  Commit: "device-mapping: Introduce DMA range map, supplanting ..."
  -- change title of commit; "bus core:" => "device-mapping:"
  -- instead of allocating the DMA map with devm, use kcalloc
 and call kfree() during device_release().  (RobH) Also,
 for three cases that want to use the same DMA map, copy
 the dma_range_map using a helper function.
  -- added a missing 'return = 0;' to of_dma_get_range().  (Nicolas)
  -- removed dma_range_overlaps(); instead return error if there
 is an existing DMA map. (Christoph).
  Commit: "PCI: brcmstb: Set additional internal memory DMA ..."
  -- Changed constant 1 to 1ULL. (Nicolas)
  Commit: "ata: ahci_brcm: Fix use of BCM7216 reset controller"
 This commit has been removed from this patchset and will be
 submitted on its own.

v9:
  Commit: "device core: Introduce DMA range map, supplanting ..."
  -- A number of code improvements were implemented as suggested by
 ChristophH.  Unfortunately, some of these changes reversed the
 implemented suggestions of other reviewers; for example, the new
 macros PFN_DMA_ADDR(), DMA_ADDR_PFN() have been pulled.

v8:
  Commit: "device core: Introduce DMA range map, supplanting ..."
  -- To satisfy a specific m68 compile configuration, I moved the 'struct
 bus_dma_region; definition out of #ifdef CONFIG_HAS_DMA and also defined
 three inline functions for !CONFIG_HAS_DMA (kernel test robot).
  -- The sunXi drivers -- suc4i_csi, sun6i_csi, cedrus_hw -- set
 a pfn_offset outside of_dma_configure() but the code offers no 
 insight on the size of the translation window.  V7 had me using
 SIZE_MAX as the size.  I have since contacted the sunXi maintainer and
 he said that using a size of SZ_4G would cover sunXi configurations.

v7:
  Commit: "device core: Introduce DMA range map, supplanting ..."
  -- remove second kcalloc/copy in device.c (AndyS)
  -- use PTR_ERR_OR_ZERO() and PHYS_PFN() (AndyS)
  -- indentation, sizeof(struct ...) => sizeof(*r) (AndyS)
  -- add pfn.h definitions: PFN_DMA_ADDR(), DMA_ADDR_PFN() (AndyS)
  -- Fixed compile error in "sun6i_csi.c" (kernel test robot)
  Commit "ata: ahci_brcm: Fix use of BCM7216 reset controller"
  -- correct name of function in the commit msg (SergeiS)
  
v6:
  Commit "device core: Introduce DMA range map":
  -- of_dma_get_range() now takes a single argument and returns either
 NULL, a valid map, or an ERR_PTR. (Robin)
  -- offsets are no longer a PFN value but an actual address. (Robin)
  -- the bus_dma_region struct stores the range size instead of
 the cpu_end and pci_end values. (Robin)
  -- devices that were setting a single offset with no boundaries
 have been modified to have boundaries; in a few places
 where this information was unavilable a /* FIXME: ... */
 comment was added. (Robin)
  -- dma_attach_offset_range() can be called when an offset
 map already exists; if it's range is already present
 nothing is done and success is returned. (Robin)
  All commits:
  -- Man name/style/corrections/etc changed (Bjorn)
  -- rebase to Torvalds master

v5:
  Commit "device core: Introduce multiple dma pfn offsets"
  -- in of/address.c: "map_size = 0" => "*map_size = 0"
  -- use kcalloc instead of kzalloc (AndyS)
  -- use PHYS_ADDR_MAX instead of "~(phys_addr_t)0"
  Commit "PCI: brcmstb: Set internal memory viewport sizes"
  -- now gives error on missing dma-ranges property.
  Commit "dt-bindings: PCI: Add bindings for more Brcmstb chips"
  -- removed "Allof:" from brcm,scb-sizes definition (RobH)
  All Commits:
  -- indentation style, use max chars 100 (AndyS)
  -- rebased to torvalds master

v4:
  Commit "device core: Introduce multiple dma pfn offsets"
  -- of_dma_get_range() does not take a dev param but instead
 takes two "out" params: map and map_size.  We do this so
 that the code that parses dma-ranges is separate from
 the code that modifies 'dev'.   (Nicolas)
  -- the separate case of having a single pfn offset has
 been removed and is now processed by going through the
 map array. (Nicolas)
  -- move attach_uniform_dma_pfn_offset() from of/address.c to
 dma/mapping.c so that it does not depend on CONFIG_OF. (Nicolas)
  -- devm_kcalloc => devm_kzalloc (DanC)
  -- add/fix assignment to 

[PATCH v11 07/11] device-mapping: Introduce DMA range map, supplanting dma_pfn_offset

2020-08-24 Thread Jim Quinlan
The new field 'dma_range_map' in struct device is used to facilitate the
use of single or multiple offsets between mapping regions of cpu addrs and
dma addrs.  It subsumes the role of "dev->dma_pfn_offset" which was only
capable of holding a single uniform offset and had no region bounds
checking.

The function of_dma_get_range() has been modified so that it takes a single
argument -- the device node -- and returns a map, NULL, or an error code.
The map is an array that holds the information regarding the DMA regions.
Each range entry contains the address offset, the cpu_start address, the
dma_start address, and the size of the region.

of_dma_configure() is the typical manner to set range offsets but there are
a number of ad hoc assignments to "dev->dma_pfn_offset" in the kernel
driver code.  These cases now invoke the function
dma_attach_offset_range(dev, cpu_addr, dma_addr, size).

Signed-off-by: Jim Quinlan 
---
 arch/arm/include/asm/dma-mapping.h| 10 +--
 arch/arm/mach-keystone/keystone.c | 17 +++--
 arch/sh/drivers/pci/pcie-sh7786.c |  9 +--
 arch/x86/pci/sta2x11-fixup.c  |  7 +-
 drivers/acpi/arm64/iort.c |  5 +-
 drivers/base/core.c   |  2 +
 drivers/gpu/drm/sun4i/sun4i_backend.c |  5 +-
 drivers/iommu/io-pgtable-arm.c|  2 +-
 .../platform/sunxi/sun4i-csi/sun4i_csi.c  |  5 +-
 .../platform/sunxi/sun6i-csi/sun6i_csi.c  |  4 +-
 drivers/of/address.c  | 72 +--
 drivers/of/device.c   | 43 ++-
 drivers/of/of_private.h   | 10 +--
 drivers/of/unittest.c | 34 ++---
 drivers/remoteproc/remoteproc_core.c  |  8 ++-
 .../staging/media/sunxi/cedrus/cedrus_hw.c|  7 +-
 drivers/usb/core/message.c|  9 ++-
 drivers/usb/core/usb.c|  7 +-
 include/linux/device.h|  4 +-
 include/linux/dma-direct.h|  8 +--
 include/linux/dma-mapping.h   | 36 ++
 kernel/dma/coherent.c | 10 +--
 kernel/dma/mapping.c  | 66 +
 23 files changed, 265 insertions(+), 115 deletions(-)

diff --git a/arch/arm/include/asm/dma-mapping.h 
b/arch/arm/include/asm/dma-mapping.h
index bdd80ddbca34..c21893f683b5 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -35,8 +35,11 @@ static inline const struct dma_map_ops 
*get_arch_dma_ops(struct bus_type *bus)
 #ifndef __arch_pfn_to_dma
 static inline dma_addr_t pfn_to_dma(struct device *dev, unsigned long pfn)
 {
-   if (dev)
-   pfn -= dev->dma_pfn_offset;
+   if (dev) {
+   phys_addr_t paddr = PFN_PHYS(pfn);
+
+   pfn -= PFN_DOWN(dma_offset_from_phys_addr(dev, paddr));
+   }
return (dma_addr_t)__pfn_to_bus(pfn);
 }
 
@@ -45,8 +48,7 @@ static inline unsigned long dma_to_pfn(struct device *dev, 
dma_addr_t addr)
unsigned long pfn = __bus_to_pfn(addr);
 
if (dev)
-   pfn += dev->dma_pfn_offset;
-
+   pfn += PFN_DOWN(dma_offset_from_dma_addr(dev, addr));
return pfn;
 }
 
diff --git a/arch/arm/mach-keystone/keystone.c 
b/arch/arm/mach-keystone/keystone.c
index 638808c4e122..78808942ad1c 100644
--- a/arch/arm/mach-keystone/keystone.c
+++ b/arch/arm/mach-keystone/keystone.c
@@ -8,6 +8,7 @@
  */
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -24,8 +25,6 @@
 
 #include "keystone.h"
 
-static unsigned long keystone_dma_pfn_offset __read_mostly;
-
 static int keystone_platform_notifier(struct notifier_block *nb,
  unsigned long event, void *data)
 {
@@ -38,9 +37,12 @@ static int keystone_platform_notifier(struct notifier_block 
*nb,
return NOTIFY_BAD;
 
if (!dev->of_node) {
-   dev->dma_pfn_offset = keystone_dma_pfn_offset;
-   dev_err(dev, "set dma_pfn_offset%08lx\n",
-   dev->dma_pfn_offset);
+   int ret = dma_set_offset_range(dev, KEYSTONE_HIGH_PHYS_START,
+  KEYSTONE_LOW_PHYS_START,
+  KEYSTONE_HIGH_PHYS_SIZE);
+   dev_err(dev, "set dma_offset%08llx%s\n",
+   KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START,
+   ret ? " failed" : "");
}
return NOTIFY_OK;
 }
@@ -51,11 +53,8 @@ static struct notifier_block platform_nb = {
 
 static void __init keystone_init(void)
 {
-   if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) {
-   keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START -
-  KEYSTONE_LOW_PHYS_START);
+   if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START)

Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Sam Ravnborg
Hi Mauro

> Before posting the big patch series again, let me send the new
> version folded into a single patch.

Review 2/N

The way output_poll_changed is used to set gpio_mux to select between
the panel and the HDMI looks strange. But I do not know if there is a
more correct way to do it. Other DRM people would need to help
here if there is a better way to do it.

I looked briefly af suspend/resume.
I think this would benefit from using drm_mode_config_helper_suspend()
and drm_mode_config_helper_resume() but I did not finalize the anlyzis.

Other than this only some small details in the following.

Sam

>  kirin9xx_drm_drv.c b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> new file mode 100644
> index ..61b65f8b1ace
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_drv.c
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hisilicon Kirin SoCs drm master driver
> + *
> + * Copyright (c) 2016 Linaro Limited.
> + * Copyright (c) 2014-2016 Hisilicon Limited.
> + * Copyright (c) 2014-2020, Huawei Technologies Co., Ltd
> + * Author:
> + *   
> + *   
> + */
> +
> +#include 
> +#include 
> +#include 
Sort includes

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
Sort includes

> +
> +#include "kirin9xx_dpe.h"
> +#include "kirin9xx_drm_drv.h"
> +
> +static int kirin_drm_kms_cleanup(struct drm_device *dev)
> +{
> + struct kirin_drm_private *priv = to_drm_private(dev);
> +
> + if (priv->fbdev)
> + priv->fbdev = NULL;
> +
> + drm_kms_helper_poll_fini(dev);
> + kirin9xx_dss_drm_cleanup(dev);
> +
> + return 0;
> +}
> +
> +static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
> +{
> + struct kirin_drm_private *priv = to_drm_private(dev);
> +
> + dsi_set_output_client(dev);
> +
> + drm_fb_helper_hotplug_event(priv->fbdev);
> +}
> +
> +static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = {
> + .fb_create = drm_gem_fb_create,
> + .output_poll_changed = kirin_fbdev_output_poll_changed,
> + .atomic_check = drm_atomic_helper_check,
> + .atomic_commit = drm_atomic_helper_commit,
> +};
> +
> +static int kirin_drm_kms_init(struct drm_device *dev)
> +{
> + long kirin_type;
> + int ret;
> +
> + dev_set_drvdata(dev->dev, dev);
> +
> + ret = drmm_mode_config_init(dev);
> + if (ret)
> + return ret;
> +
> + dev->mode_config.min_width = 0;
> + dev->mode_config.min_height = 0;
> + dev->mode_config.max_width = 2048;
> + dev->mode_config.max_height = 2048;
> + dev->mode_config.preferred_depth = 32;
> +
> + dev->mode_config.funcs = _drm_mode_config_funcs;
> +
> + /* display controller init */
> + kirin_type = (long)of_device_get_match_data(dev->dev);
> + if (WARN_ON(!kirin_type))
> + return -ENODEV;
> +
> + ret = dss_drm_init(dev, kirin_type);
> + if (ret)
> + return ret;
> +
> + /* bind and init sub drivers */
> + ret = component_bind_all(dev->dev, dev);
> + if (ret) {
> + drm_err(dev, "failed to bind all component.\n");
> + return ret;
> + }
> +
> + /* vblank init */
> + ret = drm_vblank_init(dev, dev->mode_config.num_crtc);
> + if (ret) {
> + drm_err(dev, "failed to initialize vblank.\n");
> + return ret;
> + }
> + /* with irq_enabled = true, we can use the vblank feature. */
> + dev->irq_enabled = true;
> +
> + /* reset all the states of crtc/plane/encoder/connector */
> + drm_mode_config_reset(dev);
> +
> + /* init kms poll for handling hpd */
> + drm_kms_helper_poll_init(dev);
> +
> + return 0;
> +}
> +
> +DEFINE_DRM_GEM_CMA_FOPS(kirin_drm_fops);
Move it to right above kirin_drm_driver where it is used

> +
> +static int kirin_drm_connectors_register(struct drm_device *dev)
> +{
> + struct drm_connector_list_iter conn_iter;
> + struct drm_connector *failed_connector;
> + struct drm_connector *connector;
> + int ret;
> +
> + mutex_lock(>mode_config.mutex);
> + drm_connector_list_iter_begin(dev, _iter);
> + drm_for_each_connector_iter(connector, _iter) {
> + ret = drm_connector_register(connector);
> + if (ret) {
> + failed_connector = connector;
> + goto err;
> + }
> + }
> + mutex_unlock(>mode_config.mutex);
> +
> + return 0;
> +
> +err:
> + drm_connector_list_iter_begin(dev, _iter);
> + drm_for_each_connector_iter(connector, _iter) {
> + if (failed_connector == connector)
> + break;
> + drm_connector_unregister(connector);
> + }
> + mutex_unlock(>mode_config.mutex);
> +
> + return ret;
> +}
> +
> +static struct drm_driver kirin_drm_driver = {
> + .driver_features= DRIVER_GEM | DRIVER_MODESET 

Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Sam Ravnborg
Hi Mauro



> kirin9xx_fb_panel.h b/drivers/staging/hikey9xx/gpu/kirin9xx_fb_panel.h
> new file mode 100644
> index ..a69c20470f1d
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_fb_panel.h

This file is not referenced and should be deleted.

Sam
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Dave Airlie
On Thu, 20 Aug 2020 at 20:02, Laurent Pinchart
 wrote:
>
> Hi Mauro,
>
> On Thu, Aug 20, 2020 at 09:03:26AM +0200, Mauro Carvalho Chehab wrote:
> > Em Wed, 19 Aug 2020 12:52:06 -0700 John Stultz escreveu:
> > > On Wed, Aug 19, 2020 at 8:31 AM Laurent Pinchart wrote:
> > > > On Wed, Aug 19, 2020 at 05:21:20PM +0200, Sam Ravnborg wrote:
> > > > > On Wed, Aug 19, 2020 at 01:45:28PM +0200, Mauro Carvalho Chehab wrote:
> > > > > > This patch series port the out-of-tree driver for Hikey 970 (which
> > > > > > should also support Hikey 960) from the official 96boards tree:
> > > > > >
> > > > > >https://github.com/96boards-hikey/linux/tree/hikey970-v4.9
> > > > > >
> > > > > > Based on his history, this driver seems to be originally written
> > > > > > for Kernel 4.4, and was later ported to Kernel 4.9. The original
> > > > > > driver used to depend on ION (from Kernel 4.4) and had its own
> > > > > > implementation for FB dev API.
> > > > > >
> > > > > > As I need to preserve the original history (with has patches from
> > > > > > both HiSilicon and from Linaro),  I'm starting from the original
> > > > > > patch applied there. The remaining patches are incremental,
> > > > > > and port this driver to work with upstream Kernel.
> > > > > >
> > > ...
> > > > > > - Due to legal reasons, I need to preserve the authorship of
> > > > > >   each one responsbile for each patch. So, I need to start from
> > > > > >   the original patch from Kernel 4.4;
> > > ...
> > > > > I do acknowledge you need to preserve history and all -
> > > > > but this patchset is not easy to review.
> > > >
> > > > Why do we need to preserve history ? Adding relevant Signed-off-by and
> > > > Co-developed-by should be enough, shouldn't it ? Having a public branch
> > > > that contains the history is useful if anyone is interested, but I don't
> > > > think it's required in mainline.
> > >
> > > Yea. I concur with Laurent here. I'm not sure what legal reasoning you
> > > have on this but preserving the "absolute" history here is actively
> > > detrimental for review and understanding of the patch set.
> > >
> > > Preserving Authorship, Signed-off-by lines and adding Co-developed-by
> > > lines should be sufficient to provide both atribution credit and DCO
> > > history.
> >
> > I'm not convinced that, from legal standpoint, folding things would
> > be enough. See, there are at least 3 legal systems involved here
> > among the different patch authors:
> >
> >   - civil law;
> >   - common law;
> >   - customary law + common law.
> >
> > Merging stuff altogether from different law systems can be problematic,
> > and trying to discuss this with experienced IP property lawyers will
> > for sure take a lot of time and efforts. I also bet that different
> > lawyers will have different opinions, because laws are subject to
> > interpretation. With that matter I'm not aware of any court rules
> > with regards to folded patches. So, it sounds to me that folding
> > patches is something that has yet to be proofed in courts around
> > the globe.
> >
> > At least for US legal system, it sounds that the Country of
> > origin of a patch is relevant, as they have a concept of
> > "national technology" that can be subject to export regulations.
> >
> > From my side, I really prefer to play safe and stay out of any such
> > legal discussions.
>
> Let's be serious for a moment. If you think there are legal issues in
> taking GPL-v2.0-only patches and squashing them while retaining
> authorship information through tags, the Linux kernel if *full* of that.
> You also routinely modify patches that you commit to the media subsystem
> to fix "small issues".
>
> The country of origin argument makes no sense either, the kernel code
> base if full of code coming from pretty much all country on the planet.
>
> Keeping the patches separate make this hard to review. Please squash
> them.

I'm inclined to agree with Laurent here.

Patches submitted as GPL-v2 with DCO lines and author names/companies
should be fine to be squashed and rearranged,
as long as the DCO and Authorship is kept somewhere in the new patch
that is applied.

Review is more important here.

Dave.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Sam Ravnborg
Hi Mauro.

> Before posting the big patch series again, let me send the new
> version folded into a single patch.

Review 1/N

Lots of small details I missed last time.
A good thing is that there is an opportunity to delete som more code.

Sam

> diff --git a/drivers/staging/hikey9xx/gpu/Kconfig 
> b/drivers/staging/hikey9xx/gpu/Kconfig
> new file mode 100644
> index ..8578ca953785
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/Kconfig
> @@ -0,0 +1,10 @@
> +config DRM_HISI_KIRIN9XX
> + tristate "DRM Support for Hisilicon Kirin9xx series SoCs Platform"
> + depends on DRM && OF && ARM64
> + select DRM_KMS_HELPER
> + select DRM_GEM_CMA_HELPER
> + select DRM_KMS_CMA_HELPER
> + select DRM_MIPI_DSI
> + help
> +   Choose this option if you have a HiSilicon Kirin960 or Kirin970.
> +   If M is selected the module will be called kirin9xx-drm.
> diff --git a/drivers/staging/hikey9xx/gpu/Makefile 
> b/drivers/staging/hikey9xx/gpu/Makefile
> new file mode 100644
> index ..5f7974a95367
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +kirin9xx-drm-y := kirin9xx_drm_drv.o \
> +   kirin9xx_drm_dss.o \
> +   kirin9xx_drm_dpe_utils.o \
> +   kirin970_defs.o kirin960_defs.o \
> +   kirin9xx_drm_overlay_utils.o
> +
> +obj-$(CONFIG_DRM_HISI_KIRIN9XX) += kirin9xx-drm.o kirin9xx_dw_drm_dsi.o

General comment which is true for many many Makefile's
I have never understood the love of '\'.
Something like this works equally well:

kirin9xx-drm-y := kirin9xx_drm_drv.o kirin9xx_drm_dss.o
kirin9xx-drm-y += kirin9xx_drm_dpe_utils.o kirin970_defs.o
kirin9xx-drm-y += kirin960_defs.o kirin9xx_drm_overlay_utils.o

obj-$(CONFIG_DRM_HISI_KIRIN9XX) += kirin9xx-drm.o kirin9xx_dw_drm_dsi.o


> diff --git a/drivers/staging/hikey9xx/gpu/kirin960_defs.c 
> b/drivers/staging/hikey9xx/gpu/kirin960_defs.c
> new file mode 100644
> index ..c5e1ec03c818
> --- /dev/null
> +++ b/drivers/staging/hikey9xx/gpu/kirin960_defs.c
> @@ -0,0 +1,346 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2008-2011, Hisilicon Tech. Co., Ltd. All rights reserved.
> + * Copyright (c) 2008-2020, Huawei Technologies Co., Ltd
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "kirin9xx_drm_dpe_utils.h"
> +#include "kirin9xx_drm_drv.h"
> +#include "kirin960_dpe_reg.h"
All includes blocks should be sorted.

The list of include files looks far too large for this simple file.
Reduce to the relevant include files.

> +
> +static const u32 
> kirin960_g_dss_module_base[DSS_CHN_MAX_DEFINE][MODULE_CHN_MAX] = {
> + {
> + /* D0 */
> + MIF_CH0_OFFSET,
> + AIF0_CH0_OFFSET,
> + AIF1_CH0_OFFSET,
> + MCTL_CTL_MUTEX_RCH0,
> + DSS_MCTRL_SYS_OFFSET + MCTL_RCH0_FLUSH_EN,
> + DSS_MCTRL_SYS_OFFSET + MCTL_RCH0_OV_OEN,
> + DSS_MCTRL_SYS_OFFSET + MCTL_RCH0_STARTY,
> + DSS_MCTRL_SYS_OFFSET + MCTL_MOD0_DBG,
> + DSS_RCH_D0_DMA_OFFSET,
> + DSS_RCH_D0_DFC_OFFSET,
> + 0,
> + 0,
> + 0,
> + 0,
> + 0,
> + 0,
> + DSS_RCH_D0_CSC_OFFSET,
> + }, {
...
> + },
> +};
> +
> +static const u32 
> kirin960_g_dss_module_ovl_base[DSS_MCTL_IDX_MAX][MODULE_OVL_MAX] = {
> + {
> + DSS_OVL0_OFFSET,
> + DSS_MCTRL_CTL0_OFFSET
> + }, {
> + DSS_OVL1_OFFSET,
> + DSS_MCTRL_CTL1_OFFSET
> + }, {
> + DSS_OVL2_OFFSET,
> + DSS_MCTRL_CTL2_OFFSET,
> + }, {
> + DSS_OVL3_OFFSET,
> + DSS_MCTRL_CTL3_OFFSET,
> + }, {
> + 0,
> + DSS_MCTRL_CTL4_OFFSET,
> + }, {
> + 0,
> + DSS_MCTRL_CTL5_OFFSET,
> + }
> +};
> +
> +/* SCF_LUT_CHN coef_idx */
> +static const int kirin960_g_scf_lut_chn_coef_idx[DSS_CHN_MAX_DEFINE] = {
> + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
> +};
> +
> +static const u32 
> kirin960_g_dss_module_cap[DSS_CHN_MAX_DEFINE][MODULE_CAP_MAX] = {
> + /* D2 */
> + {0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1},
> + /* D3 */
> + {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
> + /* V0 */
> + {0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1},
> + /* G0 */
> + {0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0},
> + /* V1 */
> + {0, 1, 1, 1, 0, 1, 1, 0, 1, 1, 1},
> + /* G1 */
> + {0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0},
> + /* D0 */
> + {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
> + /* D1 */
> + {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 1},
> +
> + /* W0 */
> + {1, 0, 1, 0, 0, 0, 0, 1, 0, 1, 1},
> + /* W1 */
> + {1, 0, 1, 0, 0, 0, 0, 1, 0, 1, 1},
> +
> + /* V2 */
> + {0, 1, 1, 1, 0, 1, 1, 0, 1, 1, 1},
> + /* W2 */
> 

Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Mauro Carvalho Chehab
Em Fri, 21 Aug 2020 17:56:50 +0200
Sam Ravnborg  escreveu:

> Hi Mauro.
> 
> On Fri, Aug 21, 2020 at 04:41:58PM +0200, Mauro Carvalho Chehab wrote:
> > Another quick question:
> > 
> > Em Wed, 19 Aug 2020 19:35:58 +0200
> > Sam Ravnborg  escreveu:
> >   
> > > > +#define DSS_REDUCE(x)  ((x) > 0 ? ((x) - 1) : (x))
> > > Use generic macros for this?  
> > 
> > Do you know a generic macro similar to this? Or do you mean adding
> > it to include/kernel.h?  
> 
> It looked like something there should be a macro for.
> But I do not know one.
> 
> And no, do not try to go the kernel.h route on this.
> At least not until you see more than one user.

Yeah, adding this to kernel.h just for a single usage is overkill. I would
be expecting that a non-underflow decrement logic is something that 
would be used on other places at the Kernel, but identifying this
pattern would require some time. Maybe Kernel janitors could write some
coccinelle script to replace similar patterns like that into some
macro in the future.

Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent()

2020-08-24 Thread Alex Dewar
On Mon, Aug 24, 2020 at 04:19:17PM +0100, Alex Dewar wrote:
> In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(),
> though, strangely, NULL is passed as the struct device* argument. Pass
> the UDC's device instead.
> 
> Build-tested on x86 only.
> 
> Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver")
> Signed-off-by: Alex Dewar 
> ---
> 
> So I *think* this is the right fix, but I don't have the hardware so
> I've only been able to build-test it. My worry is that I could be
> passing in the wrong struct device* here, which would squelch the
> warning without fixing the breakage.
> 
> Can someone cleverer than me tell me if this makes sense?
> 
> - Alex

PS -- I meant to put an RFC in the subject line and an extra tag:
Reported-by: Dan Carpenter 

> ---
>  drivers/staging/emxx_udc/emxx_udc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
> b/drivers/staging/emxx_udc/emxx_udc.c
> index 03929b9d3a8b..09e91449c08c 100644
> --- a/drivers/staging/emxx_udc/emxx_udc.c
> +++ b/drivers/staging/emxx_udc/emxx_udc.c
> @@ -2593,7 +2593,7 @@ static int nbu2ss_ep_queue(struct usb_ep *_ep,
>  
>   if (req->unaligned) {
>   if (!ep->virt_buf)
> - ep->virt_buf = dma_alloc_coherent(NULL, PAGE_SIZE,
> + ep->virt_buf = dma_alloc_coherent(udc->dev, PAGE_SIZE,
> >phys_buf,
> GFP_ATOMIC | GFP_DMA);
>   if (ep->epnum > 0)  {
> -- 
> 2.28.0
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent()

2020-08-24 Thread Alex Dewar
In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(),
though, strangely, NULL is passed as the struct device* argument. Pass
the UDC's device instead.

Build-tested on x86 only.

Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver")
Signed-off-by: Alex Dewar 
---

So I *think* this is the right fix, but I don't have the hardware so
I've only been able to build-test it. My worry is that I could be
passing in the wrong struct device* here, which would squelch the
warning without fixing the breakage.

Can someone cleverer than me tell me if this makes sense?

- Alex
---
 drivers/staging/emxx_udc/emxx_udc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index 03929b9d3a8b..09e91449c08c 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -2593,7 +2593,7 @@ static int nbu2ss_ep_queue(struct usb_ep *_ep,
 
if (req->unaligned) {
if (!ep->virt_buf)
-   ep->virt_buf = dma_alloc_coherent(NULL, PAGE_SIZE,
+   ep->virt_buf = dma_alloc_coherent(udc->dev, PAGE_SIZE,
  >phys_buf,
  GFP_ATOMIC | GFP_DMA);
if (ep->epnum > 0)  {
-- 
2.28.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[bug report] staging: emxx_udc: Allow for building on !ARM

2020-08-24 Thread Dan Carpenter
Hello Alex Dewar,

The patch fbcfdfab4a2c: "staging: emxx_udc: Allow for building on
!ARM" from Aug 18, 2020, leads to the following static checker
warning:

drivers/staging/emxx_udc/emxx_udc.c:2167 _nbu2ss_enable_controller() warn: 
inconsistent indenting
drivers/staging/emxx_udc/emxx_udc.c:2179 _nbu2ss_enable_controller() warn: 
inconsistent indenting
drivers/staging/emxx_udc/emxx_udc.c:2277 _nbu2ss_check_vbus() warn: 'reg_dt' 
can be either negative or positive
drivers/staging/emxx_udc/emxx_udc.c:2352 _nbu2ss_udc_irq() warn: 
'gpiod_get_value(vbus_gpio)' returns positive and negative
drivers/staging/emxx_udc/emxx_udc.c:2361 _nbu2ss_udc_irq() warn: 
'gpiod_get_value(vbus_gpio)' returns positive and negative
drivers/staging/emxx_udc/emxx_udc.c:2324 _nbu2ss_int_usb_suspend() warn: 
'reg_dt' can be either negative or positive
drivers/staging/emxx_udc/emxx_udc.c:2596 nbu2ss_ep_queue() error: NULL 
dereference inside function
drivers/staging/emxx_udc/emxx_udc.c:2795 nbu2ss_ep_fifo_flush() warn: 'data' 
can be either negative or positive
drivers/staging/emxx_udc/emxx_udc.c:3151 nbu2ss_drv_remove() error: NULL 
dereference inside function

drivers/staging/emxx_udc/emxx_udc.c
  2580  if (unlikely(!udc->driver)) {
  2581  dev_err(udc->dev, "%s, bogus device state %p\n", 
__func__,
  2582  udc->driver);
  2583  return -ESHUTDOWN;
  2584  }
  2585  
  2586  spin_lock_irqsave(>lock, flags);
  2587  
  2588  #ifdef USE_DMA
  2589  if ((uintptr_t)req->req.buf & 0x3)
  2590  req->unaligned = true;
  2591  else
  2592  req->unaligned = false;
  2593  
  2594  if (req->unaligned) {
  2595  if (!ep->virt_buf)
  2596  ep->virt_buf = dma_alloc_coherent(NULL, 
PAGE_SIZE,
  
This will oops.

  2597>phys_buf,
  2598GFP_ATOMIC | 
GFP_DMA);
  2599  if (ep->epnum > 0)  {
  2600  if (ep->direct == USB_DIR_IN)
  2601  memcpy(ep->virt_buf, req->req.buf,
  2602 req->req.length);
  2603  }
  2604  }
  2605  
  2606  if ((ep->epnum > 0) && (ep->direct == USB_DIR_OUT) &&
  2607  (req->req.dma != 0))
  2608  _nbu2ss_dma_map_single(udc, ep, req, USB_DIR_OUT);
  2609  #endif
  2610  

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 7/7] crypto: arc4 - mark ecb(arc4) skcipher as obsolete

2020-08-24 Thread Ard Biesheuvel
On Mon, 24 Aug 2020 at 15:35, Herbert Xu  wrote:
>
> On Mon, Aug 24, 2020 at 03:30:01PM +0200, Ard Biesheuvel wrote:
> >
> > +config CRYPTO_USER_ENABLE_OBSOLETE
> > + bool "Enable obsolete cryptographic algorithms for userspace"
> > + depends on CRYPTO_USER
>
> That should be CRYPTO_USER_API which is the option for af_alg.
> CRYPTO_USER is the configuration interface which has nothing to
> do with af_alg.
>

OK, will fix.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v2 7/7] crypto: arc4 - mark ecb(arc4) skcipher as obsolete

2020-08-24 Thread Herbert Xu
On Mon, Aug 24, 2020 at 03:30:01PM +0200, Ard Biesheuvel wrote:
>
> +config CRYPTO_USER_ENABLE_OBSOLETE
> + bool "Enable obsolete cryptographic algorithms for userspace"
> + depends on CRYPTO_USER

That should be CRYPTO_USER_API which is the option for af_alg.
CRYPTO_USER is the configuration interface which has nothing to
do with af_alg.

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 5/7] crypto: bcm-iproc - remove ecb(arc4) support

2020-08-24 Thread Ard Biesheuvel
Signed-off-by: Ard Biesheuvel 
---
 drivers/crypto/bcm/cipher.c | 96 +---
 drivers/crypto/bcm/cipher.h |  1 -
 drivers/crypto/bcm/spu.c| 23 +
 drivers/crypto/bcm/spu.h|  1 -
 drivers/crypto/bcm/spu2.c   | 12 +--
 drivers/crypto/bcm/spu2.h   |  1 -
 6 files changed, 6 insertions(+), 128 deletions(-)

diff --git a/drivers/crypto/bcm/cipher.c b/drivers/crypto/bcm/cipher.c
index 8a7fa1ae1ade..5d38b87b9d77 100644
--- a/drivers/crypto/bcm/cipher.c
+++ b/drivers/crypto/bcm/cipher.c
@@ -165,10 +165,6 @@ spu_skcipher_rx_sg_create(struct brcm_message *mssg,
return -EFAULT;
}
 
-   if (ctx->cipher.alg == CIPHER_ALG_RC4)
-   /* Add buffer to catch 260-byte SUPDT field for RC4 */
-   sg_set_buf(sg++, rctx->msg_buf.c.supdt_tweak, SPU_SUPDT_LEN);
-
if (stat_pad_len)
sg_set_buf(sg++, rctx->msg_buf.rx_stat_pad, stat_pad_len);
 
@@ -317,7 +313,6 @@ static int handle_skcipher_req(struct iproc_reqctx_s *rctx)
u8 local_iv_ctr[MAX_IV_SIZE];
u32 stat_pad_len;   /* num bytes to align status field */
u32 pad_len;/* total length of all padding */
-   bool update_key = false;
struct brcm_message *mssg;  /* mailbox message */
 
/* number of entries in src and dst sg in mailbox message. */
@@ -391,28 +386,6 @@ static int handle_skcipher_req(struct iproc_reqctx_s *rctx)
}
}
 
-   if (ctx->cipher.alg == CIPHER_ALG_RC4) {
-   rx_frag_num++;
-   if (chunk_start) {
-   /*
-* for non-first RC4 chunks, use SUPDT from previous
-* response as key for this chunk.
-*/
-   cipher_parms.key_buf = rctx->msg_buf.c.supdt_tweak;
-   update_key = true;
-   cipher_parms.type = CIPHER_TYPE_UPDT;
-   } else if (!rctx->is_encrypt) {
-   /*
-* First RC4 chunk. For decrypt, key in pre-built msg
-* header may have been changed if encrypt required
-* multiple chunks. So revert the key to the
-* ctx->enckey value.
-*/
-   update_key = true;
-   cipher_parms.type = CIPHER_TYPE_INIT;
-   }
-   }
-
if (ctx->max_payload == SPU_MAX_PAYLOAD_INF)
flow_log("max_payload infinite\n");
else
@@ -425,14 +398,9 @@ static int handle_skcipher_req(struct iproc_reqctx_s *rctx)
memcpy(rctx->msg_buf.bcm_spu_req_hdr, ctx->bcm_spu_req_hdr,
   sizeof(rctx->msg_buf.bcm_spu_req_hdr));
 
-   /*
-* Pass SUPDT field as key. Key field in finish() call is only used
-* when update_key has been set above for RC4. Will be ignored in
-* all other cases.
-*/
spu->spu_cipher_req_finish(rctx->msg_buf.bcm_spu_req_hdr + BCM_HDR_LEN,
   ctx->spu_req_hdr_len, !(rctx->is_encrypt),
-  _parms, update_key, chunksize);
+  _parms, chunksize);
 
atomic64_add(chunksize, _priv.bytes_out);
 
@@ -527,9 +495,6 @@ static void handle_skcipher_resp(struct iproc_reqctx_s 
*rctx)
 __func__, rctx->total_received, payload_len);
 
dump_sg(req->dst, rctx->total_received, payload_len);
-   if (ctx->cipher.alg == CIPHER_ALG_RC4)
-   packet_dump("  supdt ", rctx->msg_buf.c.supdt_tweak,
-   SPU_SUPDT_LEN);
 
rctx->total_received += payload_len;
if (rctx->total_received == rctx->total_todo) {
@@ -1853,26 +1818,6 @@ static int aes_setkey(struct crypto_skcipher *cipher, 
const u8 *key,
return 0;
 }
 
-static int rc4_setkey(struct crypto_skcipher *cipher, const u8 *key,
- unsigned int keylen)
-{
-   struct iproc_ctx_s *ctx = crypto_skcipher_ctx(cipher);
-   int i;
-
-   ctx->enckeylen = ARC4_MAX_KEY_SIZE + ARC4_STATE_SIZE;
-
-   ctx->enckey[0] = 0x00;  /* 0x00 */
-   ctx->enckey[1] = 0x00;  /* i*/
-   ctx->enckey[2] = 0x00;  /* 0x00 */
-   ctx->enckey[3] = 0x00;  /* j*/
-   for (i = 0; i < ARC4_MAX_KEY_SIZE; i++)
-   ctx->enckey[i + ARC4_STATE_SIZE] = key[i % keylen];
-
-   ctx->cipher_type = CIPHER_TYPE_INIT;
-
-   return 0;
-}
-
 static int skcipher_setkey(struct crypto_skcipher *cipher, const u8 *key,
 unsigned int keylen)
 {
@@ -1895,9 +1840,6 @@ static int skcipher_setkey(struct crypto_skcipher 
*cipher, const u8 *key,
case CIPHER_ALG_AES:
err = aes_setkey(cipher, key, keylen);
break;
-   case CIPHER_ALG_RC4:
-   err = rc4_setkey(cipher, key, keylen);
-   break;
default:

[PATCH v2 6/7] net: wireless: drop bogus CRYPTO_xxx Kconfig selects

2020-08-24 Thread Ard Biesheuvel
Drop some bogus Kconfig selects that are not entirely accurate, and
unnecessary to begin with, since the same Kconfig options also select
LIB80211 features that already imply the selected functionality (AES
for CCMP, ARC4 and ECB for TKIP)

Signed-off-by: Ard Biesheuvel 
---
 drivers/net/wireless/intel/ipw2x00/Kconfig   | 4 
 drivers/net/wireless/intersil/hostap/Kconfig | 4 
 2 files changed, 8 deletions(-)

diff --git a/drivers/net/wireless/intel/ipw2x00/Kconfig 
b/drivers/net/wireless/intel/ipw2x00/Kconfig
index b1e7b4470842..1650d5865aa0 100644
--- a/drivers/net/wireless/intel/ipw2x00/Kconfig
+++ b/drivers/net/wireless/intel/ipw2x00/Kconfig
@@ -160,11 +160,7 @@ config LIBIPW
select WIRELESS_EXT
select WEXT_SPY
select CRYPTO
-   select CRYPTO_ARC4
-   select CRYPTO_ECB
-   select CRYPTO_AES
select CRYPTO_MICHAEL_MIC
-   select CRYPTO_ECB
select CRC32
select LIB80211
select LIB80211_CRYPT_WEP
diff --git a/drivers/net/wireless/intersil/hostap/Kconfig 
b/drivers/net/wireless/intersil/hostap/Kconfig
index 6ad88299432f..c865d3156cea 100644
--- a/drivers/net/wireless/intersil/hostap/Kconfig
+++ b/drivers/net/wireless/intersil/hostap/Kconfig
@@ -5,11 +5,7 @@ config HOSTAP
select WEXT_SPY
select WEXT_PRIV
select CRYPTO
-   select CRYPTO_ARC4
-   select CRYPTO_ECB
-   select CRYPTO_AES
select CRYPTO_MICHAEL_MIC
-   select CRYPTO_ECB
select CRC32
select LIB80211
select LIB80211_CRYPT_WEP
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 7/7] crypto: arc4 - mark ecb(arc4) skcipher as obsolete

2020-08-24 Thread Ard Biesheuvel
Cryptographic algorithms may have a lifespan that is significantly
shorter than Linux's, and so we need to start phasing out algorithms
that are known to be broken, and are no longer fit for general use.

RC4 (or arc4) is a good example here: there are a few areas where its
use is still somewhat acceptable, e.g., for interoperability with legacy
wifi hardware that can only use WEP or TKIP data encryption, but that
should not imply that, for instance, use of RC4 based EAP-TLS by the WPA
supplicant for negotiating TKIP keys is equally acceptable, or that RC4
should remain available as a general purpose cryptographic transform for
all in-kernel and user space clients.

Now that all in-kernel users that need to retain support have moved to
the arc4 library interface, and the known users of ecb(arc4) via the
socket API (iwd [0] and libell [1][2]) have been updated to switch to a
local implementation, we can take the next step, and mark the ecb(arc4)
skcipher as obsolete, and only provide it if the socket API is enabled in
the first place, as well as provide the option to disable all algorithms
that have been marked as obsolete.

[0] 
https://git.kernel.org/pub/scm/network/wireless/iwd.git/commit/?id=1db8a85a60c64523
[1] https://git.kernel.org/pub/scm/libs/ell/ell.git/commit/?id=53482ce421b727c2
[2] https://git.kernel.org/pub/scm/libs/ell/ell.git/commit/?id=7f6a137809d42f6b

Signed-off-by: Ard Biesheuvel 
---
 crypto/Kconfig | 10 ++
 crypto/arc4.c  | 10 ++
 2 files changed, 20 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1b57419fa2e7..89d63d240c2e 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -136,6 +136,15 @@ config CRYPTO_USER
  Userspace configuration for cryptographic instantiations such as
  cbc(aes).
 
+config CRYPTO_USER_ENABLE_OBSOLETE
+   bool "Enable obsolete cryptographic algorithms for userspace"
+   depends on CRYPTO_USER
+   default y
+   help
+ Allow obsolete cryptographic algorithms to be selected that have
+ already been phased out from internal use by the kernel, and are
+ only useful for userspace clients that still rely on them.
+
 config CRYPTO_MANAGER_DISABLE_TESTS
bool "Disable run-time self tests"
default y
@@ -1199,6 +1208,7 @@ config CRYPTO_ANUBIS
 
 config CRYPTO_ARC4
tristate "ARC4 cipher algorithm"
+   depends on CRYPTO_USER_ENABLE_OBSOLETE
select CRYPTO_SKCIPHER
select CRYPTO_LIB_ARC4
help
diff --git a/crypto/arc4.c b/crypto/arc4.c
index aa79571dbd49..923aa7a6cd60 100644
--- a/crypto/arc4.c
+++ b/crypto/arc4.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static int crypto_arc4_setkey(struct crypto_skcipher *tfm, const u8 *in_key,
  unsigned int key_len)
@@ -39,6 +40,14 @@ static int crypto_arc4_crypt(struct skcipher_request *req)
return err;
 }
 
+static int crypto_arc4_init(struct crypto_skcipher *tfm)
+{
+   pr_warn_ratelimited("\"%s\" (%ld) uses obsolete ecb(arc4) skcipher\n",
+   current->comm, (unsigned long)current->pid);
+
+   return 0;
+}
+
 static struct skcipher_alg arc4_alg = {
/*
 * For legacy reasons, this is named "ecb(arc4)", not "arc4".
@@ -55,6 +64,7 @@ static struct skcipher_alg arc4_alg = {
.setkey =   crypto_arc4_setkey,
.encrypt=   crypto_arc4_crypt,
.decrypt=   crypto_arc4_crypt,
+   .init   =   crypto_arc4_init,
 };
 
 static int __init arc4_init(void)
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 2/7] staging/rtl8192u: switch to RC4 library interface

2020-08-24 Thread Ard Biesheuvel
Switch to the ARC4 library interface, to remove the pointless
dependency on the skcipher API, from which we will hopefully be
able to drop ecb(arc4) skcipher support.

Signed-off-by: Ard Biesheuvel 
Acked-by: Greg Kroah-Hartman 
---
 drivers/staging/rtl8192u/Kconfig  |  1 +
 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c | 81 

 drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_wep.c  | 64 +++-
 3 files changed, 27 insertions(+), 119 deletions(-)

diff --git a/drivers/staging/rtl8192u/Kconfig b/drivers/staging/rtl8192u/Kconfig
index 1edca5c304fb..ef883d462d3d 100644
--- a/drivers/staging/rtl8192u/Kconfig
+++ b/drivers/staging/rtl8192u/Kconfig
@@ -8,3 +8,4 @@ config RTL8192U
select CRYPTO
select CRYPTO_AES
select CRYPTO_CCM
+   select CRYPTO_LIB_ARC4
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c 
b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
index ffe624ed0c0c..4b415cc76715 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_crypt_tkip.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2003-2004, Jouni Malinen 
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -17,9 +18,8 @@
 
 #include "ieee80211.h"
 
+#include 
 #include 
-#include 
-   #include 
 #include 
 
 MODULE_AUTHOR("Jouni Malinen");
@@ -49,9 +49,9 @@ struct ieee80211_tkip_data {
 
int key_idx;
 
-   struct crypto_sync_skcipher *rx_tfm_arc4;
+   struct arc4_ctx rx_ctx_arc4;
+   struct arc4_ctx tx_ctx_arc4;
struct crypto_shash *rx_tfm_michael;
-   struct crypto_sync_skcipher *tx_tfm_arc4;
struct crypto_shash *tx_tfm_michael;
 
/* scratch buffers for virt_to_page() (crypto API) */
@@ -62,19 +62,14 @@ static void *ieee80211_tkip_init(int key_idx)
 {
struct ieee80211_tkip_data *priv;
 
+   if (fips_enabled)
+   return NULL;
+
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
goto fail;
priv->key_idx = key_idx;
 
-   priv->tx_tfm_arc4 = crypto_alloc_sync_skcipher("ecb(arc4)", 0, 0);
-   if (IS_ERR(priv->tx_tfm_arc4)) {
-   printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
-   "crypto API arc4\n");
-   priv->tx_tfm_arc4 = NULL;
-   goto fail;
-   }
-
priv->tx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->tx_tfm_michael)) {
printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
@@ -83,14 +78,6 @@ static void *ieee80211_tkip_init(int key_idx)
goto fail;
}
 
-   priv->rx_tfm_arc4 = crypto_alloc_sync_skcipher("ecb(arc4)", 0, 0);
-   if (IS_ERR(priv->rx_tfm_arc4)) {
-   printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
-   "crypto API arc4\n");
-   priv->rx_tfm_arc4 = NULL;
-   goto fail;
-   }
-
priv->rx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->rx_tfm_michael)) {
printk(KERN_DEBUG "ieee80211_crypt_tkip: could not allocate "
@@ -104,9 +91,7 @@ static void *ieee80211_tkip_init(int key_idx)
 fail:
if (priv) {
crypto_free_shash(priv->tx_tfm_michael);
-   crypto_free_sync_skcipher(priv->tx_tfm_arc4);
crypto_free_shash(priv->rx_tfm_michael);
-   crypto_free_sync_skcipher(priv->rx_tfm_arc4);
kfree(priv);
}
 
@@ -120,11 +105,9 @@ static void ieee80211_tkip_deinit(void *priv)
 
if (_priv) {
crypto_free_shash(_priv->tx_tfm_michael);
-   crypto_free_sync_skcipher(_priv->tx_tfm_arc4);
crypto_free_shash(_priv->rx_tfm_michael);
-   crypto_free_sync_skcipher(_priv->rx_tfm_arc4);
}
-   kfree(priv);
+   kzfree(priv);
 }
 
 
@@ -290,10 +273,8 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
u8 *pos;
struct rtl_80211_hdr_4addr *hdr;
struct cb_desc *tcb_desc = (struct cb_desc *)(skb->cb + 
MAX_DEV_ADDR_SIZE);
-   int ret = 0;
u8 rc4key[16],  *icv;
u32 crc;
-   struct scatterlist sg;
 
if (skb_headroom(skb) < 8 || skb_tailroom(skb) < 4 ||
skb->len < hdr_len)
@@ -334,21 +315,15 @@ static int ieee80211_tkip_encrypt(struct sk_buff *skb, 
int hdr_len, void *priv)
*pos++ = (tkey->tx_iv32 >> 24) & 0xff;
 
if (!tcb_desc->bHwSec) {
-   SYNC_SKCIPHER_REQUEST_ON_STACK(req, tkey->tx_tfm_arc4);
-
icv = skb_put(skb, 4);
crc = ~crc32_le(~0, pos, len);
icv[0] = crc;
icv[1] = crc >> 8;
icv[2] = crc >> 16;
icv[3] = crc >> 24;
-   

[PATCH v2 4/7] crypto: n2 - remove ecb(arc4) support

2020-08-24 Thread Ard Biesheuvel
Signed-off-by: Ard Biesheuvel 
---
 drivers/crypto/n2_core.c | 46 
 1 file changed, 46 deletions(-)

diff --git a/drivers/crypto/n2_core.c b/drivers/crypto/n2_core.c
index d8aec5153b21..8c8e17d5fb20 100644
--- a/drivers/crypto/n2_core.c
+++ b/drivers/crypto/n2_core.c
@@ -662,7 +662,6 @@ struct n2_skcipher_context {
u8  aes[AES_MAX_KEY_SIZE];
u8  des[DES_KEY_SIZE];
u8  des3[3 * DES_KEY_SIZE];
-   u8  arc4[258]; /* S-box, X, Y */
} key;
 };
 
@@ -789,36 +788,6 @@ static int n2_3des_setkey(struct crypto_skcipher 
*skcipher, const u8 *key,
return 0;
 }
 
-static int n2_arc4_setkey(struct crypto_skcipher *skcipher, const u8 *key,
- unsigned int keylen)
-{
-   struct crypto_tfm *tfm = crypto_skcipher_tfm(skcipher);
-   struct n2_skcipher_context *ctx = crypto_tfm_ctx(tfm);
-   struct n2_skcipher_alg *n2alg = n2_skcipher_alg(skcipher);
-   u8 *s = ctx->key.arc4;
-   u8 *x = s + 256;
-   u8 *y = x + 1;
-   int i, j, k;
-
-   ctx->enc_type = n2alg->enc_type;
-
-   j = k = 0;
-   *x = 0;
-   *y = 0;
-   for (i = 0; i < 256; i++)
-   s[i] = i;
-   for (i = 0; i < 256; i++) {
-   u8 a = s[i];
-   j = (j + key[k] + a) & 0xff;
-   s[i] = s[j];
-   s[j] = a;
-   if (++k >= keylen)
-   k = 0;
-   }
-
-   return 0;
-}
-
 static inline int skcipher_descriptor_len(int nbytes, unsigned int block_size)
 {
int this_len = nbytes;
@@ -1122,21 +1091,6 @@ struct n2_skcipher_tmpl {
 };
 
 static const struct n2_skcipher_tmpl skcipher_tmpls[] = {
-   /* ARC4: only ECB is supported (chaining bits ignored) */
-   {   .name   = "ecb(arc4)",
-   .drv_name   = "ecb-arc4",
-   .block_size = 1,
-   .enc_type   = (ENC_TYPE_ALG_RC4_STREAM |
-  ENC_TYPE_CHAINING_ECB),
-   .skcipher   = {
-   .min_keysize= 1,
-   .max_keysize= 256,
-   .setkey = n2_arc4_setkey,
-   .encrypt= n2_encrypt_ecb,
-   .decrypt= n2_decrypt_ecb,
-   },
-   },
-
/* DES: ECB CBC and CFB are supported */
{   .name   = "ecb(des)",
.drv_name   = "ecb-des",
-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 0/7] crypto: mark ecb(arc4) skcipher as obsolete

2020-08-24 Thread Ard Biesheuvel
RC4 hasn't aged very well, and is a poor fit for the skcipher API so it
would be good if we could get rid of the ecb(arc4) drivers in the kernel
at some point in the future. This prevents new users from creeping in, and
allows us to improve the skcipher API without having to care too much about
obsolete algorithms that may be difficult to support going forward.

So let's get rid of any remaining in-kernel users, either by switching them
to the arc4 library API (for cases which simply cannot change algorithms,
e.g., WEP), or dropping the code entirely. Also remove the remaining h/w
accelerated implementations, and mark the generic s/w implementation as
obsolete in Kconfig.

Changes since RFC [0]:
- keep ecb(arc4) generic C implementation, and the associated test vectors,
  but print a warning about ecb(arc4) being obsolete so we can identify
  remaining users
- add a Kconfig option to en/disable obsolete algorithms that are only kept
  around to prevent breaking users that rely on it via the socket interface
- add a patch to clean up some bogus Kconfig dependencies
- add acks to patches #1, #2 and #3

[0] 
https://lore.kernel.org/driverdev-devel/20200702101947.682-1-a...@kernel.org/

Cc: Herbert Xu 
Cc: "David S. Miller" 
Cc: Greg Kroah-Hartman 
Cc: Trond Myklebust 
Cc: Anna Schumaker 
Cc: "J. Bruce Fields" 
Cc: Chuck Lever 
Cc: Eric Biggers 
Cc: Arnd Bergmann 
Cc: linux-cry...@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: de...@driverdev.osuosl.org
Cc: linux-...@vger.kernel.org

Ard Biesheuvel (7):
  staging/rtl8192e: switch to RC4 library interface
  staging/rtl8192u: switch to RC4 library interface
  SUNRPC: remove RC4-HMAC-MD5 support from KerberosV
  crypto: n2 - remove ecb(arc4) support
  crypto: bcm-iproc - remove ecb(arc4) support
  net: wireless: drop bogus CRYPTO_xxx Kconfig selects
  crypto: arc4 - mark ecb(arc4) skcipher as obsolete

 crypto/Kconfig|  10 +
 crypto/arc4.c |  10 +
 drivers/crypto/bcm/cipher.c   |  96 +-
 drivers/crypto/bcm/cipher.h   |   1 -
 drivers/crypto/bcm/spu.c  |  23 +-
 drivers/crypto/bcm/spu.h  |   1 -
 drivers/crypto/bcm/spu2.c |  12 +-
 drivers/crypto/bcm/spu2.h |   1 -
 drivers/crypto/n2_core.c  |  46 ---
 drivers/net/wireless/intel/ipw2x00/Kconfig|   4 -
 drivers/net/wireless/intersil/hostap/Kconfig  |   4 -
 drivers/staging/rtl8192e/Kconfig  |   4 +-
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c  |  70 +
 drivers/staging/rtl8192e/rtllib_crypt_wep.c   |  72 +
 drivers/staging/rtl8192u/Kconfig  |   1 +
 .../rtl8192u/ieee80211/ieee80211_crypt_tkip.c |  81 +
 .../rtl8192u/ieee80211/ieee80211_crypt_wep.c  |  64 +---
 include/linux/sunrpc/gss_krb5.h   |  11 -
 include/linux/sunrpc/gss_krb5_enctypes.h  |   9 +-
 net/sunrpc/Kconfig|   1 -
 net/sunrpc/auth_gss/gss_krb5_crypto.c | 276 --
 net/sunrpc/auth_gss/gss_krb5_mech.c   |  95 --
 net/sunrpc/auth_gss/gss_krb5_seal.c   |   1 -
 net/sunrpc/auth_gss/gss_krb5_seqnum.c |  87 --
 net/sunrpc/auth_gss/gss_krb5_unseal.c |   1 -
 net/sunrpc/auth_gss/gss_krb5_wrap.c   |  65 +
 26 files changed, 97 insertions(+), 949 deletions(-)

-- 
2.17.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v2 1/7] staging/rtl8192e: switch to RC4 library interface

2020-08-24 Thread Ard Biesheuvel
Switch to the ARC4 library interface, to remove the pointless
dependency on the skcipher API, from which we will hopefully be
able to drop ecb(arc4) skcipher support.

Signed-off-by: Ard Biesheuvel 
Acked-by: Greg Kroah-Hartman 
---
 drivers/staging/rtl8192e/Kconfig |  4 +-
 drivers/staging/rtl8192e/rtllib_crypt_tkip.c | 70 ---
 drivers/staging/rtl8192e/rtllib_crypt_wep.c  | 72 
 3 files changed, 28 insertions(+), 118 deletions(-)

diff --git a/drivers/staging/rtl8192e/Kconfig b/drivers/staging/rtl8192e/Kconfig
index 1007eea6c8fc..4c440bdaaf6e 100644
--- a/drivers/staging/rtl8192e/Kconfig
+++ b/drivers/staging/rtl8192e/Kconfig
@@ -25,7 +25,7 @@ config RTLLIB_CRYPTO_CCMP
 config RTLLIB_CRYPTO_TKIP
tristate "Support for rtllib TKIP crypto"
depends on RTLLIB
-   select CRYPTO_ARC4
+   select CRYPTO_LIB_ARC4
select CRYPTO_MICHAEL_MIC
default y
help
@@ -35,7 +35,7 @@ config RTLLIB_CRYPTO_TKIP
 
 config RTLLIB_CRYPTO_WEP
tristate "Support for rtllib WEP crypto"
-   select CRYPTO_ARC4
+   select CRYPTO_LIB_ARC4
depends on RTLLIB
default y
help
diff --git a/drivers/staging/rtl8192e/rtllib_crypt_tkip.c 
b/drivers/staging/rtl8192e/rtllib_crypt_tkip.c
index 8d2a58e706d5..8c2ff37b2d3a 100644
--- a/drivers/staging/rtl8192e/rtllib_crypt_tkip.c
+++ b/drivers/staging/rtl8192e/rtllib_crypt_tkip.c
@@ -5,8 +5,9 @@
  * Copyright (c) 2003-2004, Jouni Malinen 
  */
 
+#include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -16,7 +17,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -45,9 +45,9 @@ struct rtllib_tkip_data {
u32 dot11RSNAStatsTKIPLocalMICFailures;
 
int key_idx;
-   struct crypto_sync_skcipher *rx_tfm_arc4;
+   struct arc4_ctx rx_ctx_arc4;
+   struct arc4_ctx tx_ctx_arc4;
struct crypto_shash *rx_tfm_michael;
-   struct crypto_sync_skcipher *tx_tfm_arc4;
struct crypto_shash *tx_tfm_michael;
/* scratch buffers for virt_to_page() (crypto API) */
u8 rx_hdr[16];
@@ -58,16 +58,13 @@ static void *rtllib_tkip_init(int key_idx)
 {
struct rtllib_tkip_data *priv;
 
+   if (fips_enabled)
+   return NULL;
+
priv = kzalloc(sizeof(*priv), GFP_ATOMIC);
if (priv == NULL)
goto fail;
priv->key_idx = key_idx;
-   priv->tx_tfm_arc4 = crypto_alloc_sync_skcipher("ecb(arc4)", 0, 0);
-   if (IS_ERR(priv->tx_tfm_arc4)) {
-   pr_debug("Could not allocate crypto API arc4\n");
-   priv->tx_tfm_arc4 = NULL;
-   goto fail;
-   }
 
priv->tx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->tx_tfm_michael)) {
@@ -76,13 +73,6 @@ static void *rtllib_tkip_init(int key_idx)
goto fail;
}
 
-   priv->rx_tfm_arc4 = crypto_alloc_sync_skcipher("ecb(arc4)", 0, 0);
-   if (IS_ERR(priv->rx_tfm_arc4)) {
-   pr_debug("Could not allocate crypto API arc4\n");
-   priv->rx_tfm_arc4 = NULL;
-   goto fail;
-   }
-
priv->rx_tfm_michael = crypto_alloc_shash("michael_mic", 0, 0);
if (IS_ERR(priv->rx_tfm_michael)) {
pr_debug("Could not allocate crypto API michael_mic\n");
@@ -94,9 +84,7 @@ static void *rtllib_tkip_init(int key_idx)
 fail:
if (priv) {
crypto_free_shash(priv->tx_tfm_michael);
-   crypto_free_sync_skcipher(priv->tx_tfm_arc4);
crypto_free_shash(priv->rx_tfm_michael);
-   crypto_free_sync_skcipher(priv->rx_tfm_arc4);
kfree(priv);
}
 
@@ -110,11 +98,9 @@ static void rtllib_tkip_deinit(void *priv)
 
if (_priv) {
crypto_free_shash(_priv->tx_tfm_michael);
-   crypto_free_sync_skcipher(_priv->tx_tfm_arc4);
crypto_free_shash(_priv->rx_tfm_michael);
-   crypto_free_sync_skcipher(_priv->rx_tfm_arc4);
}
-   kfree(priv);
+   kzfree(priv);
 }
 
 
@@ -289,7 +275,6 @@ static int rtllib_tkip_encrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
int ret = 0;
u8 rc4key[16],  *icv;
u32 crc;
-   struct scatterlist sg;
 
if (skb_headroom(skb) < 8 || skb_tailroom(skb) < 4 ||
skb->len < hdr_len)
@@ -331,8 +316,6 @@ static int rtllib_tkip_encrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
*pos++ = (tkey->tx_iv32 >> 24) & 0xff;
 
if (!tcb_desc->bHwSec) {
-   SYNC_SKCIPHER_REQUEST_ON_STACK(req, tkey->tx_tfm_arc4);
-
icv = skb_put(skb, 4);
crc = ~crc32_le(~0, pos, len);
icv[0] = crc;
@@ -340,15 +323,8 @@ static int rtllib_tkip_encrypt(struct sk_buff *skb, int 
hdr_len, void *priv)
icv[2] = crc >> 16;
icv[3] = crc >> 24;
 
-   sg_init_one(, pos, len+4);

[PATCH v2 3/7] SUNRPC: remove RC4-HMAC-MD5 support from KerberosV

2020-08-24 Thread Ard Biesheuvel
The RC4-HMAC-MD5 KerberosV algorithm is based on RFC 4757 [0], which
was specifically issued for interoperability with Windows 2000, but was
never intended to receive the same level of support. The RFC says

  The IETF Kerberos community supports publishing this specification as
  an informational document in order to describe this widely
  implemented technology.  However, while these encryption types
  provide the operations necessary to implement the base Kerberos
  specification [RFC4120], they do not provide all the required
  operations in the Kerberos cryptography framework [RFC3961].  As a
  result, it is not generally possible to implement potential
  extensions to Kerberos using these encryption types.  The Kerberos
  encryption type negotiation mechanism [RFC4537] provides one approach
  for using such extensions even when a Kerberos infrastructure uses
  long-term RC4 keys.  Because this specification does not implement
  operations required by RFC 3961 and because of security concerns with
  the use of RC4 and MD4 discussed in Section 8, this specification is
  not appropriate for publication on the standards track.

  The RC4-HMAC encryption types are used to ease upgrade of existing
  Windows NT environments, provide strong cryptography (128-bit key
  lengths), and provide exportable (meet United States government
  export restriction requirements) encryption.  This document describes
  the implementation of those encryption types.

Furthermore, this RFC was re-classified as 'historic' by RFC 8429 [1] in
2018, stating that 'none of the encryption types it specifies should be
used'

Note that other outdated algorithms are left in place (some of which are
guarded by CONFIG_SUNRPC_DISABLE_INSECURE_ENCTYPES), so this should only
adversely affect interoperability with Windows NT/2000 systems that have
not received any updates since 2008 (but are connected to a network
nonetheless)

[0] https://tools.ietf.org/html/rfc4757
[1] https://tools.ietf.org/html/rfc8429

Signed-off-by: Ard Biesheuvel 
Acked-by: J. Bruce Fields 
---
 include/linux/sunrpc/gss_krb5.h  |  11 -
 include/linux/sunrpc/gss_krb5_enctypes.h |   9 +-
 net/sunrpc/Kconfig   |   1 -
 net/sunrpc/auth_gss/gss_krb5_crypto.c| 276 
 net/sunrpc/auth_gss/gss_krb5_mech.c  |  95 ---
 net/sunrpc/auth_gss/gss_krb5_seal.c  |   1 -
 net/sunrpc/auth_gss/gss_krb5_seqnum.c|  87 --
 net/sunrpc/auth_gss/gss_krb5_unseal.c|   1 -
 net/sunrpc/auth_gss/gss_krb5_wrap.c  |  65 +
 9 files changed, 16 insertions(+), 530 deletions(-)

diff --git a/include/linux/sunrpc/gss_krb5.h b/include/linux/sunrpc/gss_krb5.h
index e8f8ffe7448b..91f43d86879d 100644
--- a/include/linux/sunrpc/gss_krb5.h
+++ b/include/linux/sunrpc/gss_krb5.h
@@ -141,14 +141,12 @@ enum sgn_alg {
SGN_ALG_MD2_5 = 0x0001,
SGN_ALG_DES_MAC = 0x0002,
SGN_ALG_3 = 0x0003, /* not published */
-   SGN_ALG_HMAC_MD5 = 0x0011,  /* microsoft w2k; no support */
SGN_ALG_HMAC_SHA1_DES3_KD = 0x0004
 };
 enum seal_alg {
SEAL_ALG_NONE = 0x,
SEAL_ALG_DES = 0x,
SEAL_ALG_1 = 0x0001,/* not published */
-   SEAL_ALG_MICROSOFT_RC4 = 0x0010,/* microsoft w2k; no support */
SEAL_ALG_DES3KD = 0x0002
 };
 
@@ -316,14 +314,5 @@ gss_krb5_aes_decrypt(struct krb5_ctx *kctx, u32 offset, 
u32 len,
 struct xdr_buf *buf, u32 *plainoffset,
 u32 *plainlen);
 
-int
-krb5_rc4_setup_seq_key(struct krb5_ctx *kctx,
-  struct crypto_sync_skcipher *cipher,
-  unsigned char *cksum);
-
-int
-krb5_rc4_setup_enc_key(struct krb5_ctx *kctx,
-  struct crypto_sync_skcipher *cipher,
-  s32 seqnum);
 void
 gss_krb5_make_confounder(char *p, u32 conflen);
diff --git a/include/linux/sunrpc/gss_krb5_enctypes.h 
b/include/linux/sunrpc/gss_krb5_enctypes.h
index 981c89cef19d..87eea679d750 100644
--- a/include/linux/sunrpc/gss_krb5_enctypes.h
+++ b/include/linux/sunrpc/gss_krb5_enctypes.h
@@ -13,15 +13,13 @@
 #ifdef CONFIG_SUNRPC_DISABLE_INSECURE_ENCTYPES
 
 /*
- * NB: This list includes encryption types that were deprecated
- * by RFC 8429 (DES3_CBC_SHA1 and ARCFOUR_HMAC).
+ * NB: This list includes DES3_CBC_SHA1, which was deprecated by RFC 8429.
  *
  * ENCTYPE_AES256_CTS_HMAC_SHA1_96
  * ENCTYPE_AES128_CTS_HMAC_SHA1_96
  * ENCTYPE_DES3_CBC_SHA1
- * ENCTYPE_ARCFOUR_HMAC
  */
-#define KRB5_SUPPORTED_ENCTYPES "18,17,16,23"
+#define KRB5_SUPPORTED_ENCTYPES "18,17,16"
 
 #else  /* CONFIG_SUNRPC_DISABLE_INSECURE_ENCTYPES */
 
@@ -32,12 +30,11 @@
  * ENCTYPE_AES256_CTS_HMAC_SHA1_96
  * ENCTYPE_AES128_CTS_HMAC_SHA1_96
  * ENCTYPE_DES3_CBC_SHA1
- * ENCTYPE_ARCFOUR_HMAC
  * ENCTYPE_DES_CBC_MD5
  * ENCTYPE_DES_CBC_CRC
  * ENCTYPE_DES_CBC_MD4
  */
-#define KRB5_SUPPORTED_ENCTYPES "18,17,16,23,3,1,2"
+#define KRB5_SUPPORTED_ENCTYPES "18,17,16,3,1,2"
 
 #endif /* 

Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Mauro Carvalho Chehab
Em Mon, 24 Aug 2020 08:49:30 +0200
Mauro Carvalho Chehab  escreveu:

> Hi John,
> 
> Em Wed, 19 Aug 2020 20:28:44 -0700
> John Stultz  escreveu:
> 
> 
> > That said even with the patches I've got on top of your series, I
> > still see a few issues:
> > 1) I'm seeing red-blue swap with your driver.  I need to dig a bit to
> > see what the difference is, I know gralloc has a config option for
> > this, and maybe the version of the driver I'm carrying has it wrong?  
> 
> Maybe it is due to this:
> 
>   drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c:  hal_fmt 
> = HISI_FB_PIXEL_FORMAT_BGRA_;/* dss_get_format(fb->pixel_format); */
> 
> It sounds to me that someone added a hack hardcoding BGRA_ over
> there.
> 
> Btw, I removed the hack, with:
> 
> 
> diff --git a/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c 
> b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c
> index a68db1a27bbf..ba64aae371e4 100644
> --- a/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c
> +++ b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c
> @@ -857,7 +857,7 @@ void hisi_fb_pan_display(struct drm_plane *plane)
> rect.right = src_w - 1;
> rect.top = 0;
> rect.bottom = src_h - 1;
> -   hal_fmt = HISI_FB_PIXEL_FORMAT_BGRA_;/* 
> dss_get_format(fb->pixel_format); */
> +   hal_fmt = dss_get_format(fb->format->format);
>  
> DRM_DEBUG_DRIVER("channel%d: src:(%d,%d, %dx%d) crtc:(%d,%d, %dx%d), 
> rect(%d,%d,%d,%d),fb:%dx%d, pixel_format=%d, stride=%d, paddr=0x%x, 
> bpp=%d.\n",
>  chn_idx, src_x, src_y, src_w, src_h,
> 
> 
> And now red and blue are swapped on my HDMI screen too.
> 
> I'll compare this part with your version, but I guess the bug is
> on this logic.

It sounds to me that the Hikey 960 version on your tree has some color 
inversion hack, just for ARGB 32 bpp. See:

static const struct kirin_format dpe_formats[] = {
{ DRM_FORMAT_RGB565, DPE_RGB_565 },
{ DRM_FORMAT_BGR565, DPE_BGR_565 },
{ DRM_FORMAT_XRGB, DPE_RGBX_ },
{ DRM_FORMAT_XBGR, DPE_BGRX_ },
{ DRM_FORMAT_RGBA, DPE_RGBA_ },
{ DRM_FORMAT_BGRA, DPE_BGRA_ },
{ DRM_FORMAT_ARGB, DPE_BGRA_ },
{ DRM_FORMAT_ABGR, DPE_RGBA_ },
};

The last two lines are weird, as they're reverting the byte order,
If there's some endiannes issue (which the change from ARGB->ABGR 
seems to imply), I would expect to have something similar for the 
other formats as well.

I did some tests here: both FB and X11 sets bpp to 24 bits.

Trying to use "startx -- -depth 32" don't work:

"Default Screen Section" for depth/fbbpp 32/32
[   129.250] (EE) modeset(0): Given depth (32) is not supported by the 
driver

Which sounds weird, as the driver announces 32 bit formats. 
I suspect that this could be related to the valid modes hack at 
the driver.

Btw, there are some color shift also with --depth 16, but replacing
BGR <=> RGB didn't work.

Did you test the different bpp resolutions with the driver on your
tree? The enclosed patch makes 24 bits bpp work here.

Thanks,
Mauro

-

[PATCH] staging: kirin9xx/gpu: rework the colorspace mode setting logic

There are some problems when setting the fourcc KMS:
The original code hardcodes BGRA_. The real issue here seems
to be that the byte order is different than the one for Kirin 620.

Instead of addressing it, the origincla code just used a fixed
mode. A port of the Hikey 960 DPE driver based on Kernel 5.0,
found at:


https://git.linaro.org/people/john.stultz/android-dev.git/tree/drivers/gpu/drm/hisilicon/kirin/kirin_drm_dpe.c?h=dev/hikey960-mainline-WIP

contains a hack that swaps the byte order for 32-bits
ARGB/BRGR (see the last two lines):

{ DRM_FORMAT_RGB565, DPE_RGB_565 },
{ DRM_FORMAT_BGR565, DPE_BGR_565 },
{ DRM_FORMAT_XRGB, DPE_RGBX_ },
{ DRM_FORMAT_XBGR, DPE_BGRX_ },
{ DRM_FORMAT_RGBA, DPE_RGBA_ },
{ DRM_FORMAT_BGRA, DPE_BGRA_ },
{ DRM_FORMAT_ARGB, DPE_BGRA_ },
{ DRM_FORMAT_ABGR, DPE_RGBA_ },

But the same change was not applied to other modesets.

The Hikey 960 port was tested with AOSP, which seems to
be using ABGR format. Here, the chosen fourcc was
XBGR 32 bpp instead. I suspect that the original developer
also found a similar issue and decided to hardcode the
fourcc format.

That's said, currently the drivers uses some code instead of
tables in order to seek for the register settings. The version
from John Stultz tre for Kirin 960 that does a better approach
of using tables instead of code.

I opted to use the same code as the basis for the new logic,
as it makes easier to identify what the driver is actually doing.


Re: [PATCH 01/12] staging: wfx: fix BA when device is AP and MFP is enabled

2020-08-24 Thread Jérôme Pouiller
On Monday 24 August 2020 11:50:42 CEST Dan Carpenter wrote:
> On Thu, Aug 20, 2020 at 05:58:47PM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller 
> >
> > The protection of the management frames is mainly done by mac80211.
> > However, frames for the management of the BlockAck sessions are directly
> > sent by the device. These frames have to be protected if MFP is in use.
> > So the driver has to pass the MFP configuration to the device.
> >
> > Until now, the BlockAck management frames were completely unprotected
> > whatever the status of the MFP negotiation. So, some devices dropped
> > these frames.
> >
> > The device has two knobs to control the MFP. One global and one per
> > station. Normally, the driver should always enable global MFP. Then it
> > should enable MFP on every station with which MFP was successfully
> > negotiated. Unfortunately, the older firmwares only provide the
> > global control.
> >
> > So, this patch enable global MFP as it is exposed in the beacon. Then it
> > marks every station with which the MFP is effective.
> >
> > Thus, the support for the old firmwares is not so bad. It may only
> > encounter some difficulties to negotiate BA sessions when the local
> > device (the AP) is MFP capable (ieee80211w=1) but the station is not.
> > The only solution for this case is to upgrade the firmware.
> >
> > Signed-off-by: Jérôme Pouiller 
> > ---
> >  drivers/staging/wfx/sta.c | 22 +-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> > index ad63332f690c..9c1c8223a49f 100644
> > --- a/drivers/staging/wfx/sta.c
> > +++ b/drivers/staging/wfx/sta.c
> > @@ -434,7 +434,7 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct 
> > ieee80211_vif *vif,
> >   wvif->link_id_map |= BIT(sta_priv->link_id);
> >   WARN_ON(!sta_priv->link_id);
> >   WARN_ON(sta_priv->link_id >= HIF_LINK_ID_MAX);
> > - hif_map_link(wvif, sta->addr, 0, sta_priv->link_id);
> > + hif_map_link(wvif, sta->addr, sta->mfp ? 2 : 0, sta_priv->link_id);
> >
> >   return 0;
> >  }
> > @@ -474,6 +474,25 @@ static int wfx_upload_ap_templates(struct wfx_vif 
> > *wvif)
> >   return 0;
> >  }
> >
> > +static void wfx_set_mfp_ap(struct wfx_vif *wvif)
> > +{
> > + struct sk_buff *skb = ieee80211_beacon_get(wvif->wdev->hw, wvif->vif);
> > + const int ieoffset = offsetof(struct ieee80211_mgmt, 
> > u.beacon.variable);
> > + const u16 *ptr = (u16 *)cfg80211_find_ie(WLAN_EID_RSN,
> > +  skb->data + ieoffset,
> > +  skb->len - ieoffset);
> > + const int pairwise_cipher_suite_count_offset = 8 / sizeof(u16);
> > + const int pairwise_cipher_suite_size = 4 / sizeof(u16);
> > + const int akm_suite_size = 4 / sizeof(u16);
> > +
> > + if (ptr) {
> > + ptr += pairwise_cipher_suite_count_offset;
> > + ptr += 1 + pairwise_cipher_suite_size * *ptr;
> 
> The value of "*ptr" comes from skb->data.  How do we know that it
> doesn't point to something beyond the end of the skb->data buffer?

I think the beacon come from hostapd (or any userspace application with
the necessary permissions). Indeed, it could be corrupted.

I have noticed that WLAN_EID_RSN is parsed at multiple places in the
kernel and I haven't seen any particular check :( (and WLAN_EID_RSN is
probably not the only dangerous IE).

Anyway, I am going to add a few checks on values of ptr.

> > + ptr += 1 + akm_suite_size * *ptr;
> > + hif_set_mfp(wvif, *ptr & BIT(7), *ptr & BIT(6));
> > + }
> > +}

-- 
Jérôme Pouiller


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 2/2] staging: android: Remove BUG from ion_system_heap.c

2020-08-24 Thread Dan Carpenter
On Mon, Aug 24, 2020 at 02:27:08PM +0300, Dan Carpenter wrote:
> On Mon, Aug 24, 2020 at 02:24:57PM +0300, Dan Carpenter wrote:
> > On Fri, Aug 21, 2020 at 09:25:26AM -0700, Randy Dunlap wrote:
> > > On 8/21/20 8:28 AM, Tomer Samara wrote:
> > > > Remove BUG() from ion_sytem_heap.c
> > > > 
> > > > this fix the following checkpatch issue:
> > > > Avoid crashing the kernel - try using WARN_ON &
> > > > recovery code ratherthan BUG() or BUG_ON().
> > > > 
> > > > Signed-off-by: Tomer Samara 
> > > > ---
> > > >  drivers/staging/android/ion/ion_system_heap.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> > > > b/drivers/staging/android/ion/ion_system_heap.c
> > > > index eac0632ab4e8..00d6154aec34 100644
> > > > --- a/drivers/staging/android/ion/ion_system_heap.c
> > > > +++ b/drivers/staging/android/ion/ion_system_heap.c
> > > > @@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
> > > > for (i = 0; i < NUM_ORDERS; i++)
> > > > if (order == orders[i])
> > > > return i;
> > > > -   BUG();
> > > > +   /* This is impossible. */
> > > > return -1;
> > > >  }
> > > 
> > > Hi,
> > > Please explain why this is impossible.
> > > 
> > > If some caller calls order_to_index(5), it will return -1, yes?
> > > 
> > 
> > I was happy enough with the comment as-is given that I suggested it.
> > But an alternative comment could be "/* This is impossible.
> > We always pass valid values to this function. */
> 
> Another option is to just change the BUG_ON() to a WARN_ON().  I feel
> like that communicates the same thing but makes checkpatch happy.

Actually earlier Greg pointed out that some systems have panic on warn
so WARN_ON() doesn't work.  Just add the comment.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Immediate Attention.

2020-08-24 Thread Mr. Luini
GIANNI ORIGONI AVVOCATOS (IT)
Partners Piazza Belgioioso,10, 
20121 Milano MI, Italia 
Email: gianni.orig...@bk.ru.
Telefono: 00393512842679.
 
Sir/Madam,
 
I am Giovanni Francesco Luini, a member of GIANNI ORIGONI AVVOCATOS (IT) and a 
private lawyer to Sr. Vittorio Gregotti (deceased). I was his attorney and 
legal adviser for over a decade, until his death four months ago. Sr. Vittorio 
Gregotti lives from 10 August 1927 / 15 March 2020) was an Italian architect, 
born in Novara Piedmont, Italy. He was seen as both a member of the Neo-Avant 
Garde and a key figure in 1970s Postmodernism. He was 92 years old when he died 
of coronavirus along with his heart disease. Before retiring from active 
duties, he was a successful businessman and architect.  He had no biological 
children and due to this reason and his generosity, he decided to share his 
entire property and money that worth € 30,000,000.000 (thirty billion euros) to 
relatives/individuals and company owners. However, he did not declare all his 
assets intentionally, leaving behind € 25,000,000.00(twenty-five million 
euros), deposited in two different bank account outside Italy which is 
confidential between him and my law firm.
 
He placed his will and documents under the jurisdictions of my law firm, and 
with the documents, we have here your name appeared as one of the beneficiaries 
of a cash amount of € 15, 685,000.00. while   € 9, 315,000.00, and the 
properties in the south of France are allocated to  Sr Vittorio Gregotti 
FONDAZIONI for maintenance and financing. 

For more information, you can visit his Wikipedia page: 
https://en.wikipedia.org/wiki/Vittorio_Gregotti .
 
The fund (€ 15,685,000,00) is ready to be transferred to you on the following 
conditions:
 
* It is necessary to issue this chamber the progress report of the investments 
on a quarterly basis, that is, every three months.

* Any more investment development plans be forwarded to this firm for preview 
before execution.

* You are in charge of managing and directing investments, while our duty is to 
oversee and verify that the investment is in stock or flow.
 
*We will send delegates to inspect the investment after this Covid-19 after the 
funds have been transferred to you and two months later for evaluation. Since 
this is the measure that was endorsed by the deceased to ensure beneficiaries 
make the right investment and management of the fund they received.
 
If you have read these conditions and have decided on them, then you can 
proceed to the correct fund transfer. This will involve me processing some 
legal documents and affidavits to seal our contract and affidavits that back up 
the transfer of funds. Then the transfer of funds can be done to your bank 
account. 
To begin this process, send a photocopy of a valid identity either your 
driver's license or your international traveling passport.
 

Note: From this stage, all correspondence must be sent to my private email box: 
gianni.orig...@bk.ru 

With best regards,
 
Giovanni Francesco Luini (Q.C.)
C.E.O. GIANNI ORIGONI AVVOCATOS (IT).
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 2/2] staging: android: Remove BUG from ion_system_heap.c

2020-08-24 Thread Dan Carpenter
On Mon, Aug 24, 2020 at 02:24:57PM +0300, Dan Carpenter wrote:
> On Fri, Aug 21, 2020 at 09:25:26AM -0700, Randy Dunlap wrote:
> > On 8/21/20 8:28 AM, Tomer Samara wrote:
> > > Remove BUG() from ion_sytem_heap.c
> > > 
> > > this fix the following checkpatch issue:
> > > Avoid crashing the kernel - try using WARN_ON &
> > > recovery code ratherthan BUG() or BUG_ON().
> > > 
> > > Signed-off-by: Tomer Samara 
> > > ---
> > >  drivers/staging/android/ion/ion_system_heap.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> > > b/drivers/staging/android/ion/ion_system_heap.c
> > > index eac0632ab4e8..00d6154aec34 100644
> > > --- a/drivers/staging/android/ion/ion_system_heap.c
> > > +++ b/drivers/staging/android/ion/ion_system_heap.c
> > > @@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
> > >   for (i = 0; i < NUM_ORDERS; i++)
> > >   if (order == orders[i])
> > >   return i;
> > > - BUG();
> > > + /* This is impossible. */
> > >   return -1;
> > >  }
> > 
> > Hi,
> > Please explain why this is impossible.
> > 
> > If some caller calls order_to_index(5), it will return -1, yes?
> > 
> 
> I was happy enough with the comment as-is given that I suggested it.
> But an alternative comment could be "/* This is impossible.
> We always pass valid values to this function. */

Another option is to just change the BUG_ON() to a WARN_ON().  I feel
like that communicates the same thing but makes checkpatch happy.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 2/2] staging: android: Remove BUG from ion_system_heap.c

2020-08-24 Thread Dan Carpenter
On Fri, Aug 21, 2020 at 09:25:26AM -0700, Randy Dunlap wrote:
> On 8/21/20 8:28 AM, Tomer Samara wrote:
> > Remove BUG() from ion_sytem_heap.c
> > 
> > this fix the following checkpatch issue:
> > Avoid crashing the kernel - try using WARN_ON &
> > recovery code ratherthan BUG() or BUG_ON().
> > 
> > Signed-off-by: Tomer Samara 
> > ---
> >  drivers/staging/android/ion/ion_system_heap.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/android/ion/ion_system_heap.c 
> > b/drivers/staging/android/ion/ion_system_heap.c
> > index eac0632ab4e8..00d6154aec34 100644
> > --- a/drivers/staging/android/ion/ion_system_heap.c
> > +++ b/drivers/staging/android/ion/ion_system_heap.c
> > @@ -30,7 +30,7 @@ static int order_to_index(unsigned int order)
> > for (i = 0; i < NUM_ORDERS; i++)
> > if (order == orders[i])
> > return i;
> > -   BUG();
> > +   /* This is impossible. */
> > return -1;
> >  }
> 
> Hi,
> Please explain why this is impossible.
> 
> If some caller calls order_to_index(5), it will return -1, yes?
> 

I was happy enough with the comment as-is given that I suggested it.
But an alternative comment could be "/* This is impossible.
We always pass valid values to this function. */

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v4 1/2] staging: android: Remove BUG_ON from ion_page_pool.c

2020-08-24 Thread Dan Carpenter
On Fri, Aug 21, 2020 at 06:27:37PM +0300, Tomer Samara wrote:
> BUG_ON() is removed at ion_page_pool.c
> 
> Fixes the following issue:
> Avoid crashing the kernel - try using WARN_ON & recovery code ratherthan 
> BUG() or BUG_ON().
> 
> Signed-off-by: Tomer Samara 
> ---

You should put a note here about what changed between v3 vs v4. Like so:
---
v4: Just remove the BUG_ON()s instead of adding new returns.
v3: Hand the new return paths or whatever.

Anyway, looks good.

Reviewed-by: Dan Carpenter 

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: rtl8192e: fix missing failure check on a call to dev_alloc_name

2020-08-24 Thread Dan Carpenter
On Fri, Aug 21, 2020 at 02:15:12PM +0100, Colin King wrote:
> From: Colin Ian King 
> 
> Currently the second call to dev_alloc_name is not checking if this
> failed.  Add the check and perform necessary cleanup on an error.
> 
> Addresses-Coverity: ("Unchecked return value")
> Fixes: 94a799425eee ("rtl8192e: Import new version of driver from realtek")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c 
> b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> index fac58eebf263..7b15faeefff2 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
> @@ -2489,7 +2489,8 @@ static int _rtl92e_pci_probe(struct pci_dev *pdev,
>   if (dev_alloc_name(dev, ifname) < 0) {
^^

>   RT_TRACE(COMP_INIT,
>"Oops: devname already taken! Trying wlan%%d...\n");
  ^^^

> - dev_alloc_name(dev, ifname);
^^
> + if (dev_alloc_name(dev, ifname) < 0)

The "ifname" is wrong.  It was intended to be "wlan%d" or something like
the comment says.  It will always fail.  One potential is to just delete
this retest and assume that the user will fix their mistake and try
again.  That's probably the best solution in fact.

> + goto err_unmap;
>   }

if (dev_alloc_name(dev, ifname) < 0) {
RT_TRACE(COMP_INIT, "Oops: devname failed '%s'!\n", ifname);
goto err_unmap;
}

>  
>   RT_TRACE(COMP_INIT, "Driver probe completed1\n");

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 01/12] staging: wfx: fix BA when device is AP and MFP is enabled

2020-08-24 Thread Dan Carpenter
On Thu, Aug 20, 2020 at 05:58:47PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller 
> 
> The protection of the management frames is mainly done by mac80211.
> However, frames for the management of the BlockAck sessions are directly
> sent by the device. These frames have to be protected if MFP is in use.
> So the driver has to pass the MFP configuration to the device.
> 
> Until now, the BlockAck management frames were completely unprotected
> whatever the status of the MFP negotiation. So, some devices dropped
> these frames.
> 
> The device has two knobs to control the MFP. One global and one per
> station. Normally, the driver should always enable global MFP. Then it
> should enable MFP on every station with which MFP was successfully
> negotiated. Unfortunately, the older firmwares only provide the
> global control.
> 
> So, this patch enable global MFP as it is exposed in the beacon. Then it
> marks every station with which the MFP is effective.
> 
> Thus, the support for the old firmwares is not so bad. It may only
> encounter some difficulties to negotiate BA sessions when the local
> device (the AP) is MFP capable (ieee80211w=1) but the station is not.
> The only solution for this case is to upgrade the firmware.
> 
> Signed-off-by: Jérôme Pouiller 
> ---
>  drivers/staging/wfx/sta.c | 22 +-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
> index ad63332f690c..9c1c8223a49f 100644
> --- a/drivers/staging/wfx/sta.c
> +++ b/drivers/staging/wfx/sta.c
> @@ -434,7 +434,7 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct 
> ieee80211_vif *vif,
>   wvif->link_id_map |= BIT(sta_priv->link_id);
>   WARN_ON(!sta_priv->link_id);
>   WARN_ON(sta_priv->link_id >= HIF_LINK_ID_MAX);
> - hif_map_link(wvif, sta->addr, 0, sta_priv->link_id);
> + hif_map_link(wvif, sta->addr, sta->mfp ? 2 : 0, sta_priv->link_id);
>  
>   return 0;
>  }
> @@ -474,6 +474,25 @@ static int wfx_upload_ap_templates(struct wfx_vif *wvif)
>   return 0;
>  }
>  
> +static void wfx_set_mfp_ap(struct wfx_vif *wvif)
> +{
> + struct sk_buff *skb = ieee80211_beacon_get(wvif->wdev->hw, wvif->vif);
> + const int ieoffset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
> + const u16 *ptr = (u16 *)cfg80211_find_ie(WLAN_EID_RSN,
> +  skb->data + ieoffset,
> +  skb->len - ieoffset);
> + const int pairwise_cipher_suite_count_offset = 8 / sizeof(u16);
> + const int pairwise_cipher_suite_size = 4 / sizeof(u16);
> + const int akm_suite_size = 4 / sizeof(u16);
> +
> + if (ptr) {
> + ptr += pairwise_cipher_suite_count_offset;
> + ptr += 1 + pairwise_cipher_suite_size * *ptr;

The value of "*ptr" comes from skb->data.  How do we know that it
doesn't point to something beyond the end of the skb->data buffer?

> + ptr += 1 + akm_suite_size * *ptr;
> + hif_set_mfp(wvif, *ptr & BIT(7), *ptr & BIT(6));
> + }
> +}

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: greybus: fix warnings detected by sparse

2020-08-24 Thread Dan Carpenter
On Mon, Aug 24, 2020 at 10:50:59AM +0800, Coiby Xu wrote:
> This patch fix the following warnings from sparse,
> 
> $ make C=2 drivers/staging/greybus/
> drivers/staging/greybus/audio_codec.c:691:36: warning: incorrect type in 
> initializer (different base types)
> drivers/staging/greybus/audio_codec.c:691:36:expected unsigned long long 
> [usertype] formats
> drivers/staging/greybus/audio_codec.c:691:36:got restricted 
> snd_pcm_format_t [usertype]
> drivers/staging/greybus/audio_codec.c:701:36: warning: incorrect type in 
> initializer (different base types)
> drivers/staging/greybus/audio_codec.c:701:36:expected unsigned long long 
> [usertype] formats
> drivers/staging/greybus/audio_codec.c:701:36:got restricted 
> snd_pcm_format_t [usertype]
> drivers/staging/greybus/audio_module.c:222:25: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_module.c:222:25:expected restricted __le16 
> [usertype] data_cport
> drivers/staging/greybus/audio_module.c:222:25:got unsigned short 
> [usertype] intf_cport_id
> drivers/staging/greybus/audio_topology.c:460:40: warning: restricted __le32 
> degrades to integer
> drivers/staging/greybus/audio_topology.c:691:41: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:691:41:expected unsigned int 
> access
> drivers/staging/greybus/audio_topology.c:691:41:got restricted __le32 
> [usertype] access
> drivers/staging/greybus/audio_topology.c:746:44: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:746:44:expected unsigned int
> drivers/staging/greybus/audio_topology.c:746:44:got restricted __le32
> drivers/staging/greybus/audio_topology.c:748:52: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:748:52:expected unsigned int
> drivers/staging/greybus/audio_topology.c:748:52:got restricted __le32
> drivers/staging/greybus/audio_topology.c:802:42: warning: restricted __le32 
> degrades to integer
> drivers/staging/greybus/audio_topology.c:805:50: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:805:50:expected restricted __le32
> drivers/staging/greybus/audio_topology.c:805:50:got unsigned int
> drivers/staging/greybus/audio_topology.c:814:50: warning: restricted __le32 
> degrades to integer
> drivers/staging/greybus/audio_topology.c:817:58: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:817:58:expected restricted __le32
> drivers/staging/greybus/audio_topology.c:817:58:got unsigned int
> drivers/staging/greybus/audio_topology.c:889:25: warning: incorrect type in 
> assignment (different base types)
> drivers/staging/greybus/audio_topology.c:889:25:expected unsigned int 
> access
> drivers/staging/greybus/audio_topology.c:889:25:got restricted __le32 
> [usertype] access
> 
> Signed-off-by: Coiby Xu 
> ---
>  drivers/staging/greybus/audio_codec.c|  4 ++--
>  drivers/staging/greybus/audio_module.c   |  2 +-
>  drivers/staging/greybus/audio_topology.c | 18 +-
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_codec.c 
> b/drivers/staging/greybus/audio_codec.c
> index 74538f8c5fa4..494aa823e998 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -688,7 +688,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
>   .playback = {
>   .stream_name = "I2S 0 Playback",
>   .rates = SNDRV_PCM_RATE_48000,
> - .formats = SNDRV_PCM_FORMAT_S16_LE,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE,
>   .rate_max = 48000,
>   .rate_min = 48000,
>   .channels_min = 1,
> @@ -698,7 +698,7 @@ static struct snd_soc_dai_driver gbaudio_dai[] = {
>   .capture = {
>   .stream_name = "I2S 0 Capture",
>   .rates = SNDRV_PCM_RATE_48000,
> - .formats = SNDRV_PCM_FORMAT_S16_LE,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE,
>   .rate_max = 48000,
>   .rate_min = 48000,
>   .channels_min = 1,

These changes need to be explained better.  We're changing formats from
2 to 1 << 2.

When you're writing commit messages, please imagine me as the target
audience.  I have a fairly decent understanding of the kernel and C, but
I don't know very much about the sound subsystem.

This code used to work, right?  How was it that changing a 2 to a 4
makes it better?  It needs to be explained in the commit message.  This
change probably needs to be split into a separate commit because it
seems different from the rest 

Re: [PATCH] drivers: staging: comedi: fixed duplicate words from checkpatch

2020-08-24 Thread Ian Abbott

On 22/08/2020 21:21, Ethan Edwards wrote:

Fixed various different checkpatch duplicate word warnings, the TODO
file said to fix checkpatch warnings.

My old email didn't CC the mailing lists, ignore the old one, sorry.


Everything above the '---' line ends up in the git commit message 
(unless edited out manually, and Greg hates that).  The comment about 
CC'ing email lists doesn't belong in the commit message.  Could you send 
a '[PATCH v2]' with that line edited out and place a description of the 
v2 changes just below the '---' line?


Thanks.



Signed-off-by: Ethan Edwards 
---
  drivers/staging/comedi/comedi.h  | 4 ++--
  drivers/staging/comedi/comedidev.h   | 2 +-
  drivers/staging/comedi/drivers/addi_apci_1564.c  | 4 ++--
  drivers/staging/comedi/drivers/comedi_8255.c | 2 +-
  drivers/staging/comedi/drivers/ni_tiocmd.c   | 2 +-
  drivers/staging/comedi/drivers/pcmuio.c  | 2 +-
  drivers/staging/comedi/drivers/quatech_daqp_cs.c | 2 +-
  7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index 09a940066c0e..b5d00a006dbb 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -680,7 +680,7 @@ struct comedi_rangeinfo {
   * value of 1 volt.
   *
   * The only defined flag value is %RF_EXTERNAL (%0x100), indicating that the
- * the range needs to be multiplied by an external reference.
+ * range needs to be multiplied by an external reference.
   */
  struct comedi_krange {
int min;
@@ -970,7 +970,7 @@ enum i8254_mode {
   *   major reasons exist why this caused major confusion for users:
   *   1) The register values are _NOT_ in user documentation, but rather in
   * arcane locations, such as a few register programming manuals that are
- * increasingly hard to find and the NI MHDDK (comments in in example 
code).
+ * increasingly hard to find and the NI MHDDK (comments in example code).
   * There is no one place to find the various valid values of the 
registers.
   *   2) The register values are _NOT_ completely consistent.  There is no way 
to
   * gain any sense of intuition of which values, or even enums one should 
use
diff --git a/drivers/staging/comedi/comedidev.h 
b/drivers/staging/comedi/comedidev.h
index 0dff1ac057cd..0e1b95ef9a4d 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -627,7 +627,7 @@ extern const struct comedi_lrange range_unknown;
   * @range: Array of  comedi_krange, one for each range.
   *
   * Each element of @range[] describes the minimum and maximum physical range
- * range and the type of units.  Typically, the type of unit is %UNIT_volt
+ * and the type of units.  Typically, the type of unit is %UNIT_volt
   * (i.e. volts) and the minimum and maximum are in millionths of a volt.
   * There may also be a flag that indicates the minimum and maximum are merely
   * scale factors for an unknown, external reference.
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index fadefcb5c237..06fc7ed96200 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -544,7 +544,7 @@ static int apci1564_timer_insn_write(struct comedi_device 
*dev,
  {
struct apci1564_private *devpriv = dev->private;
  
-	/* just write the last last to the reload register */

+   /* just write the last to the reload register */
if (insn->n) {
unsigned int val = data[insn->n - 1];
  
@@ -628,7 +628,7 @@ static int apci1564_counter_insn_write(struct comedi_device *dev,

unsigned int chan = CR_CHAN(insn->chanspec);
unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
  
-	/* just write the last last to the reload register */

+   /* just write the last to the reload register */
if (insn->n) {
unsigned int val = data[insn->n - 1];
  
diff --git a/drivers/staging/comedi/drivers/comedi_8255.c b/drivers/staging/comedi/drivers/comedi_8255.c

index 3298725b9ba5..b7ca465933ee 100644
--- a/drivers/staging/comedi/drivers/comedi_8255.c
+++ b/drivers/staging/comedi/drivers/comedi_8255.c
@@ -248,7 +248,7 @@ EXPORT_SYMBOL_GPL(subdev_8255_mm_init);
   * subdev_8255_regbase - get offset of 8255 registers or call-back context
   * @s: comedi subdevice
   *
- * Returns the 'regbase' parameter that was previously passed to to
+ * Returns the 'regbase' parameter that was previously passed to
   * subdev_8255_init() or subdev_8255_mm_init() to set up the subdevice.
   * Only valid if the subdevice was set up successfully.
   */
diff --git a/drivers/staging/comedi/drivers/ni_tiocmd.c 
b/drivers/staging/comedi/drivers/ni_tiocmd.c
index 2a9f7e9821a7..ab6d9e8269f3 100644
--- a/drivers/staging/comedi/drivers/ni_tiocmd.c
+++ b/drivers/staging/comedi/drivers/ni_tiocmd.c
@@ -286,7 +286,7 @@ int 

Re: [PATCH 00/49] DRM driver for Hikey 970

2020-08-24 Thread Mauro Carvalho Chehab
Hi John,

Em Wed, 19 Aug 2020 20:28:44 -0700
John Stultz  escreveu:


> That said even with the patches I've got on top of your series, I
> still see a few issues:
> 1) I'm seeing red-blue swap with your driver.  I need to dig a bit to
> see what the difference is, I know gralloc has a config option for
> this, and maybe the version of the driver I'm carrying has it wrong?

Maybe it is due to this:

drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c:  hal_fmt 
= HISI_FB_PIXEL_FORMAT_BGRA_;/* dss_get_format(fb->pixel_format); */

It sounds to me that someone added a hack hardcoding BGRA_ over
there.

Btw, I removed the hack, with:


diff --git a/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c 
b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c
index a68db1a27bbf..ba64aae371e4 100644
--- a/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c
+++ b/drivers/staging/hikey9xx/gpu/kirin9xx_drm_overlay_utils.c
@@ -857,7 +857,7 @@ void hisi_fb_pan_display(struct drm_plane *plane)
rect.right = src_w - 1;
rect.top = 0;
rect.bottom = src_h - 1;
-   hal_fmt = HISI_FB_PIXEL_FORMAT_BGRA_;/* 
dss_get_format(fb->pixel_format); */
+   hal_fmt = dss_get_format(fb->format->format);
 
DRM_DEBUG_DRIVER("channel%d: src:(%d,%d, %dx%d) crtc:(%d,%d, %dx%d), 
rect(%d,%d,%d,%d),fb:%dx%d, pixel_format=%d, stride=%d, paddr=0x%x, bpp=%d.\n",
 chn_idx, src_x, src_y, src_w, src_h,


And now red and blue are swapped on my HDMI screen too.

I'll compare this part with your version, but I guess the bug is
on this logic.


Thanks,
Mauro
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel