Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Mar 2017 17:09:26 -0600
From:      Ian Lepore <ian@freebsd.org>
To:        Marius Strobl <marius@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r315466 - in head/sys/dev: mmc sdhci
Message-ID:  <1489792166.40576.192.camel@freebsd.org>
In-Reply-To: <201703172257.v2HMvbXp078389@repo.freebsd.org>
References:  <201703172257.v2HMvbXp078389@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 2017-03-17 at 22:57 +0000, Marius Strobl wrote:
> Author: marius
> Date: Fri Mar 17 22:57:37 2017
> New Revision: 315466
> URL: https://svnweb.freebsd.org/changeset/base/315466
> 
> Log:
>   Again, fixes regarding style(4), to comments, includes and unused
>   parameters.
> 
> Modified:
>   head/sys/dev/mmc/bridge.h
>   head/sys/dev/mmc/mmc.c
>   head/sys/dev/mmc/mmcbr_if.m
>   head/sys/dev/sdhci/sdhci.c
>   head/sys/dev/sdhci/sdhci_acpi.c
>   head/sys/dev/sdhci/sdhci_if.m
>   head/sys/dev/sdhci/sdhci_pci.c
> 
> Modified: head/sys/dev/mmc/bridge.h
> =====================================================================
> =========
> --- head/sys/dev/mmc/bridge.h	Fri Mar 17 22:02:02 2017	
> (r315465)
> +++ head/sys/dev/mmc/bridge.h	Fri Mar 17 22:57:37 2017	
> (r315466)
> @@ -111,7 +111,7 @@ enum mmc_bus_timing {
>  
>  struct mmc_ios {
>  	uint32_t	clock;	/* Speed of the clock in Hz to
> move data */
> -	enum mmc_vdd	vdd;	/* Voltage to apply to the
> power pins/ */
> +	enum mmc_vdd	vdd;	/* Voltage to apply to the
> power pins */
>  	enum mmc_bus_mode bus_mode;
>  	enum mmc_chip_select chip_select;
>  	enum mmc_bus_width bus_width;
> 
> Modified: head/sys/dev/mmc/mmc.c
> =====================================================================
> =========
> --- head/sys/dev/mmc/mmc.c	Fri Mar 17 22:02:02 2017	(r3
> 15465)
> +++ head/sys/dev/mmc/mmc.c	Fri Mar 17 22:57:37 2017	(r3
> 15466)
> @@ -792,6 +792,7 @@ mmc_get_bits(uint32_t *bits, int bit_len
>  	const int i = (bit_len / 32) - (start / 32) - 1;
>  	const int shift = start & 31;
>  	uint32_t retval = bits[i] >> shift;
> +
>  	if (size + shift > 32)
>  		retval |= bits[i - 1] << (32 - shift);
>  	return (retval & ((1llu << size) - 1));
> @@ -1464,7 +1465,7 @@ mmc_rescan_cards(struct mmc_softc *sc)
>  		return;
>  	for (i = 0; i < devcount; i++) {
>  		ivar = device_get_ivars(devlist[i]);
> -		if (mmc_select_card(sc, ivar->rca)) {
> +		if (mmc_select_card(sc, ivar->rca) != MMC_ERR_NONE)
> {
>  			if (bootverbose || mmc_debug)
>  				device_printf(sc->dev,
>  				    "Card at relative address %d
> lost.\n",
> 
> Modified: head/sys/dev/mmc/mmcbr_if.m
> =====================================================================
> =========
> --- head/sys/dev/mmc/mmcbr_if.m	Fri Mar 17 22:02:02 2017	
> (r315465)
> +++ head/sys/dev/mmc/mmcbr_if.m	Fri Mar 17 22:57:37 2017	
> (r315466)
> @@ -65,8 +65,9 @@
>  INTERFACE mmcbr;
>  
>  #
> -# Called by the mmcbus to setup the IO pins correctly, the voltage
> to use
> -# for the card, the type of selects, power modes and bus width.
> +# Called by the mmcbus to set up the IO pins correctly, the
> common/core
> +# supply voltage (VDD/VCC) to use for the device, the clock
> frequency, the
> +# type of SPI chip select, power mode and bus width.
>  #
>  METHOD int update_ios {
>  	device_t	brdev;
> @@ -76,8 +77,8 @@ METHOD int update_ios {
>  #
>  # Called by the mmcbus or its children to schedule a mmc
> request.  These
>  # requests are queued.  Time passes.  The bridge then gets
> notification
> -# of the status of request, who then notifies the requesting device
> via
> -# the xfer_done mmcbus method.
> +# of the status of the request, who then notifies the requesting
> device
> +# by calling the completion function supplied as part of the
> request.
>  #
>  METHOD int request {
>  	device_t	brdev;
> 
> Modified: head/sys/dev/sdhci/sdhci.c
> =====================================================================
> =========
> --- head/sys/dev/sdhci/sdhci.c	Fri Mar 17 22:02:02 2017	
> (r315465)
> +++ head/sys/dev/sdhci/sdhci.c	Fri Mar 17 22:57:37 2017	
> (r315466)
> @@ -309,9 +309,8 @@ sdhci_set_clock(struct sdhci_slot *slot,
>  		}
>  		/* Divider 1:1 is 0x00, 2:1 is 0x01, 256:1 is 0x80
> ... */
>  		div >>= 1;
> -	}
> -	else {
> -		/* Version 3.0 divisors are multiples of two up to
> 1023*2 */
> +	} else {
> +		/* Version 3.0 divisors are multiples of two up to
> 1023 * 2 */
>  		if (clock >= clk_base)
>  			div = 0;
>  		else {
> @@ -1349,7 +1348,7 @@ sdhci_data_irq(struct sdhci_slot *slot, 
>  	if (intmask & SDHCI_INT_DMA_END) {
>  		data = slot->curcmd->data;
>  
> -		/* Unload DMA buffer... */
> +		/* Unload DMA buffer ... */

All this churn in the driver for meaningless style fixes and __unused
markup and so on is bad enough, especially considering that you know
people are doing out-of-tree work on this and have to deal with all the
 merge conflicts this is causing, but changes to comments that actually
change correct grammar and punctuation to incorrect, such as this, just
serves to turn annoying into infuriating.

-- Ian


>  		left = data->len - slot->offset;
>  		if (data->flags & MMC_DATA_READ) {
>  			bus_dmamap_sync(slot->dmatag, slot->dmamap,
> 
> Modified: head/sys/dev/sdhci/sdhci_acpi.c
> =====================================================================
> =========
> --- head/sys/dev/sdhci/sdhci_acpi.c	Fri Mar 17 22:02:02 2017	
> (r315465)
> +++ head/sys/dev/sdhci/sdhci_acpi.c	Fri Mar 17 22:57:37 2017	
> (r315466)
> @@ -87,7 +87,8 @@ static void sdhci_acpi_intr(void *arg);
>  static int sdhci_acpi_detach(device_t dev);
>  
>  static uint8_t
> -sdhci_acpi_read_1(device_t dev, struct sdhci_slot *slot, bus_size_t
> off)
> +sdhci_acpi_read_1(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
>  
> @@ -97,8 +98,8 @@ sdhci_acpi_read_1(device_t dev, struct s
>  }
>  
>  static void
> -sdhci_acpi_write_1(device_t dev, struct sdhci_slot *slot, bus_size_t
> off,
> -    uint8_t val)
> +sdhci_acpi_write_1(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off, uint8_t val)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
>  
> @@ -108,7 +109,8 @@ sdhci_acpi_write_1(device_t dev, struct 
>  }
>  
>  static uint16_t
> -sdhci_acpi_read_2(device_t dev, struct sdhci_slot *slot, bus_size_t
> off)
> +sdhci_acpi_read_2(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
>  
> @@ -118,8 +120,8 @@ sdhci_acpi_read_2(device_t dev, struct s
>  }
>  
>  static void
> -sdhci_acpi_write_2(device_t dev, struct sdhci_slot *slot, bus_size_t
> off,
> -    uint16_t val)
> +sdhci_acpi_write_2(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off, uint16_t val)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
>  
> @@ -129,7 +131,8 @@ sdhci_acpi_write_2(device_t dev, struct 
>  }
>  
>  static uint32_t
> -sdhci_acpi_read_4(device_t dev, struct sdhci_slot *slot, bus_size_t
> off)
> +sdhci_acpi_read_4(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
>  
> @@ -139,8 +142,8 @@ sdhci_acpi_read_4(device_t dev, struct s
>  }
>  
>  static void
> -sdhci_acpi_write_4(device_t dev, struct sdhci_slot *slot, bus_size_t
> off,
> -    uint32_t val)
> +sdhci_acpi_write_4(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off, uint32_t val)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
>  
> @@ -150,7 +153,7 @@ sdhci_acpi_write_4(device_t dev, struct 
>  }
>  
>  static void
> -sdhci_acpi_read_multi_4(device_t dev, struct sdhci_slot *slot,
> +sdhci_acpi_read_multi_4(device_t dev, struct sdhci_slot *slot
> __unused,
>      bus_size_t off, uint32_t *data, bus_size_t count)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
> @@ -159,7 +162,7 @@ sdhci_acpi_read_multi_4(device_t dev, st
>  }
>  
>  static void
> -sdhci_acpi_write_multi_4(device_t dev, struct sdhci_slot *slot,
> +sdhci_acpi_write_multi_4(device_t dev, struct sdhci_slot *slot
> __unused,
>      bus_size_t off, uint32_t *data, bus_size_t count)
>  {
>  	struct sdhci_acpi_softc *sc = device_get_softc(dev);
> 
> Modified: head/sys/dev/sdhci/sdhci_if.m
> =====================================================================
> =========
> --- head/sys/dev/sdhci/sdhci_if.m	Fri Mar 17 22:02:02 2017	
> (r315465)
> +++ head/sys/dev/sdhci/sdhci_if.m	Fri Mar 17 22:57:37 2017	
> (r315466)
> @@ -59,22 +59,13 @@
>  #
>  
>  #include <sys/types.h>
> -#include <sys/systm.h>
> -#include <sys/lock.h>
> -#include <sys/mutex.h>
> +#include <sys/bus.h>
>  #include <sys/sysctl.h>
>  #include <sys/taskqueue.h>
>  
> -#include <machine/bus.h>
> -
>  #include <dev/mmc/bridge.h>
> -#include <dev/mmc/mmcreg.h>
>  #include <dev/sdhci/sdhci.h>
>  
> -CODE {
> -	struct sdhci_slot;
> -}
> -
>  INTERFACE sdhci;
>  
>  METHOD uint8_t read_1 {
> 
> Modified: head/sys/dev/sdhci/sdhci_pci.c
> =====================================================================
> =========
> --- head/sys/dev/sdhci/sdhci_pci.c	Fri Mar 17 22:02:02 2017	
> (r315465)
> +++ head/sys/dev/sdhci/sdhci_pci.c	Fri Mar 17 22:57:37 2017	
> (r315466)
> @@ -147,7 +147,7 @@ SYSCTL_INT(_hw_sdhci, OID_AUTO, enable_m
>      0, "Enable MSI interrupts");
>  
>  static uint8_t
> -sdhci_pci_read_1(device_t dev, struct sdhci_slot *slot, bus_size_t
> off)
> +sdhci_pci_read_1(device_t dev, struct sdhci_slot *slot __unused,
> bus_size_t off)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
>  
> @@ -157,8 +157,8 @@ sdhci_pci_read_1(device_t dev, struct sd
>  }
>  
>  static void
> -sdhci_pci_write_1(device_t dev, struct sdhci_slot *slot, bus_size_t
> off,
> -    uint8_t val)
> +sdhci_pci_write_1(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off, uint8_t val)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
>  
> @@ -168,7 +168,7 @@ sdhci_pci_write_1(device_t dev, struct s
>  }
>  
>  static uint16_t
> -sdhci_pci_read_2(device_t dev, struct sdhci_slot *slot, bus_size_t
> off)
> +sdhci_pci_read_2(device_t dev, struct sdhci_slot *slot __unused,
> bus_size_t off)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
>  
> @@ -178,8 +178,8 @@ sdhci_pci_read_2(device_t dev, struct sd
>  }
>  
>  static void
> -sdhci_pci_write_2(device_t dev, struct sdhci_slot *slot, bus_size_t
> off,
> -    uint16_t val)
> +sdhci_pci_write_2(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off, uint16_t val)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
>  
> @@ -189,7 +189,7 @@ sdhci_pci_write_2(device_t dev, struct s
>  }
>  
>  static uint32_t
> -sdhci_pci_read_4(device_t dev, struct sdhci_slot *slot, bus_size_t
> off)
> +sdhci_pci_read_4(device_t dev, struct sdhci_slot *slot __unused,
> bus_size_t off)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
>  
> @@ -199,8 +199,8 @@ sdhci_pci_read_4(device_t dev, struct sd
>  }
>  
>  static void
> -sdhci_pci_write_4(device_t dev, struct sdhci_slot *slot, bus_size_t
> off,
> -    uint32_t val)
> +sdhci_pci_write_4(device_t dev, struct sdhci_slot *slot __unused,
> +    bus_size_t off, uint32_t val)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
>  
> @@ -210,7 +210,7 @@ sdhci_pci_write_4(device_t dev, struct s
>  }
>  
>  static void
> -sdhci_pci_read_multi_4(device_t dev, struct sdhci_slot *slot,
> +sdhci_pci_read_multi_4(device_t dev, struct sdhci_slot *slot
> __unused,
>      bus_size_t off, uint32_t *data, bus_size_t count)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
> @@ -219,7 +219,7 @@ sdhci_pci_read_multi_4(device_t dev, str
>  }
>  
>  static void
> -sdhci_pci_write_multi_4(device_t dev, struct sdhci_slot *slot,
> +sdhci_pci_write_multi_4(device_t dev, struct sdhci_slot *slot
> __unused,
>      bus_size_t off, uint32_t *data, bus_size_t count)
>  {
>  	struct sdhci_pci_softc *sc = device_get_softc(dev);
> 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1489792166.40576.192.camel>