Date: Mon, 17 Feb 2020 19:29:15 +0100 From: Dimitry Andric <dim@FreeBSD.org> To: Scott Long <scottl@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head@freebsd.org Subject: Re: svn commit: r357647 - in head/sys: cam/ata cam/mmc cam/nvme cam/scsi dev/aac dev/altera/avgen dev/altera/sdcard dev/amr dev/cfi dev/flash dev/ida dev/ips dev/mfi dev/mlx dev/mmc dev/nvme dev/pst de... Message-ID: <E677722E-4AAA-4BD9-A555-4FB532C66CBA@FreeBSD.org> In-Reply-To: <202002070922.0179M9up051074@repo.freebsd.org> References: <202002070922.0179M9up051074@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_D8BE2C6E-0B9E-40D8-804A-4923A420B97E Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii On 7 Feb 2020, at 10:22, Scott Long <scottl@freebsd.org> wrote: >=20 > Author: scottl > Date: Fri Feb 7 09:22:08 2020 > New Revision: 357647 > URL: https://svnweb.freebsd.org/changeset/base/357647 >=20 > Log: > Ever since the block layer expanded its command syntax beyond just > BIO_READ and BIO_WRITE, we've handled this expanded syntax poorly in > drivers when the driver doesn't support a particular command. Do a > sweep and fix that. ... > Modified: head/sys/dev/altera/sdcard/altera_sdcard_io.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- head/sys/dev/altera/sdcard/altera_sdcard_io.c Fri Feb 7 = 08:39:00 2020 (r357646) > +++ head/sys/dev/altera/sdcard/altera_sdcard_io.c Fri Feb 7 = 09:22:08 2020 (r357647) > @@ -293,27 +293,27 @@ recheck: > } >=20 > static void > -altera_sdcard_io_start_internal(struct altera_sdcard_softc *sc, = struct bio *bp) > +altera_sdcard_io_start_internal(struct altera_sdcard_softc *sc, = struct bio **bp) > { >=20 > - switch (bp->bio_cmd) { > + switch (*bp->bio_cmd) { > case BIO_READ: > - altera_sdcard_write_cmd_arg(sc, bp->bio_pblkno * > + altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno * > ALTERA_SDCARD_SECTORSIZE); > altera_sdcard_write_cmd(sc, = ALTERA_SDCARD_CMD_READ_BLOCK); > break; >=20 > case BIO_WRITE: > - altera_sdcard_write_rxtx_buffer(sc, bp->bio_data, > - bp->bio_bcount); > - altera_sdcard_write_cmd_arg(sc, bp->bio_pblkno * > + altera_sdcard_write_rxtx_buffer(sc, *bp->bio_data, > + *bp->bio_bcount); > + altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno * > ALTERA_SDCARD_SECTORSIZE); > altera_sdcard_write_cmd(sc, = ALTERA_SDCARD_CMD_WRITE_BLOCK); > break; >=20 > default: > - panic("%s: unsupported I/O operation %d", __func__, > - bp->bio_cmd); > + biofinish(*bp, NULL, EOPNOTSUPP); > + *bp =3D NULL; > } > } >=20 This gets the indirections wrong, leading to errors with at least clang = 10: sys/dev/altera/sdcard/altera_sdcard_io.c: In function = 'altera_sdcard_io_start_internal': sys/dev/altera/sdcard/altera_sdcard_io.c:299:13: error: '*bp' is a = pointer; did you mean to use '->'? switch (*bp->bio_cmd) { ^~ -> sys/dev/altera/sdcard/altera_sdcard_io.c:301:38: error: '*bp' is a = pointer; did you mean to use '->'? altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno * ^~ -> sys/dev/altera/sdcard/altera_sdcard_io.c:307:42: error: '*bp' is a = pointer; did you mean to use '->'? altera_sdcard_write_rxtx_buffer(sc, *bp->bio_data, ^~ -> sys/dev/altera/sdcard/altera_sdcard_io.c:308:10: error: '*bp' is a = pointer; did you mean to use '->'? *bp->bio_bcount); ^~ -> sys/dev/altera/sdcard/altera_sdcard_io.c:309:38: error: '*bp' is a = pointer; did you mean to use '->'? altera_sdcard_write_cmd_arg(sc, *bp->bio_pblkno * ^~ -> In this case, (*bp)->fieldname is the correct way to specify the struct = fields. > @@ -332,8 +332,8 @@ altera_sdcard_io_start(struct altera_sdcard_softc = *sc, > */ > KASSERT(bp->bio_bcount =3D=3D ALTERA_SDCARD_SECTORSIZE, > ("%s: I/O size not %d", __func__, = ALTERA_SDCARD_SECTORSIZE)); > - altera_sdcard_io_start_internal(sc, bp); > - sc->as_currentbio =3D bp; > + altera_sdcard_io_start_internal(sc, &bp); > + sc->as_currentbio =3D *bp; > sc->as_retriesleft =3D ALTERA_SDCARD_RETRY_LIMIT; > } And a similar error here: sys/dev/altera/sdcard/altera_sdcard_io.c: In function = 'altera_sdcard_io_start': sys/dev/altera/sdcard/altera_sdcard_io.c:336:20: error: incompatible = types when assigning to type 'struct bio *' from type 'struct bio' sc->as_currentbio =3D *bp; ^ As sc->as_currentbio is already a pointer, there is no need to = dereference bp again. I submitted https://reviews.freebsd.org/D23730 to get this fixed. -Dimitry --Apple-Mail=_D8BE2C6E-0B9E-40D8-804A-4923A420B97E Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=signature.asc Content-Type: application/pgp-signature; name=signature.asc Content-Description: Message signed with OpenPGP -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.2 iF0EARECAB0WIQR6tGLSzjX8bUI5T82wXqMKLiCWowUCXkrbewAKCRCwXqMKLiCW o/K3AKCUnzPm0IfBvYH85zHlI2CBvXm9sACfSqibE6We0VCDmVKvkaFagMqw2mw= =xBA8 -----END PGP SIGNATURE----- --Apple-Mail=_D8BE2C6E-0B9E-40D8-804A-4923A420B97E--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E677722E-4AAA-4BD9-A555-4FB532C66CBA>