Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 18 Mar 2017 00:39:47 +0100
From:      Marius Strobl <marius@FreeBSD.org>
To:        Ian Lepore <ian@freebsd.org>
Cc:        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:  <20170317233947.GJ39477@alchemy.franken.de>
In-Reply-To: <1489792166.40576.192.camel@freebsd.org>
References:  <201703172257.v2HMvbXp078389@repo.freebsd.org> <1489792166.40576.192.camel@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Mar 17, 2017 at 05:09:26PM -0600, Ian Lepore wrote:
> 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.
> 

Hrm, could you please tell me were I changed correct grammar into
incorrect one? In the examples you quoted above I can't spot such
a case but introducing incorrect grammar certainly wasn't the intent.

As for the out-of-tree work, I still haven't been told what's the branch
that actually is used for MMCCAM; the branch imp@ mentioned last time
was deleted shortly after. As I said before, my best guess, therefore,
is mmccam-refactoring-rebased in kibab's tree, but that one is even still
missing functional changes to sdhci(4) you did. Nevertheless, looking
at that branch, as for sdhci(4) mostly new sdhci_cam_*() functions
have been added but hardly any existing code was touched, so there
should be only a few conflicts, if any at all. And as was determined
before, mmc(4) and mmcsd(4) aren't touched at all by the MMCCAM work.
That said, I'm more than willing to help with resolving merge conflicts
I'm causing, if someone tells me the right tree and branch ...

Marius




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