From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Kleine-Budde Subject: Re: [PATCH v7 4/7] can: provide a separate bittiming_const parameter to bittiming functions Date: Fri, 28 Feb 2014 11:46:51 +0100 Message-ID: <5310691B.1090507@pengutronix.de> References: <1393583188-13307-1-git-send-email-socketcan@hartkopp.net> <1393583188-13307-5-git-send-email-socketcan@hartkopp.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="B5AtSnNBA9MMgC687oKId2KxeBrG36mRE" Return-path: Received: from metis.ext.pengutronix.de ([92.198.50.35]:42400 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751342AbaB1Kq5 (ORCPT ); Fri, 28 Feb 2014 05:46:57 -0500 In-Reply-To: <1393583188-13307-5-git-send-email-socketcan@hartkopp.net> Sender: linux-can-owner@vger.kernel.org List-ID: To: Oliver Hartkopp , linux-can@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --B5AtSnNBA9MMgC687oKId2KxeBrG36mRE Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 02/28/2014 11:26 AM, Oliver Hartkopp wrote: > As the bittiming calculation functions are to be used with different > bittiming_const structures for CAN and CAN FD the direct reference to > priv->bittiming_const inside these functions has to be removed. >=20 > Also simplify the return value generation in can_get_bittiming() as onl= y > correct return values of can_[calc|fixup]_bittiming() lead to a return = value > of zero. And moved the check for existing bittiming const to one place.= >=20 > Signed-off-by: Oliver Hartkopp > --- > drivers/net/can/dev.c | 59 ++++++++++++++++++++++---------------------= -------- > 1 file changed, 25 insertions(+), 34 deletions(-) >=20 > diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c > index 89fc798..c8cd528 100644 > --- a/drivers/net/can/dev.c > +++ b/drivers/net/can/dev.c > @@ -99,10 +99,10 @@ static int can_update_spt(const struct can_bittimin= g_const *btc, > return 1000 * (tseg + 1 - *tseg2) / (tseg + 1); > } > =20 > -static int can_calc_bittiming(struct net_device *dev, struct can_bitti= ming *bt) > +static int can_calc_bittiming(struct net_device *dev, struct can_bitti= ming *bt, > + const struct can_bittiming_const *btc) > { > struct can_priv *priv =3D netdev_priv(dev); > - const struct can_bittiming_const *btc =3D priv->bittiming_const; > long rate, best_rate =3D 0; > long best_error =3D 1000000000, error =3D 0; > int best_tseg =3D 0, best_brp =3D 0, brp =3D 0; > @@ -110,9 +110,6 @@ static int can_calc_bittiming(struct net_device *de= v, struct can_bittiming *bt) > int spt_error =3D 1000, spt =3D 0, sampl_pt; > u64 v64; > =20 > - if (!priv->bittiming_const) > - return -ENOTSUPP; > - > /* Use CIA recommended sample points */ > if (bt->sample_point) { > sampl_pt =3D bt->sample_point; > @@ -204,7 +201,8 @@ static int can_calc_bittiming(struct net_device *de= v, struct can_bittiming *bt) > return 0; > } > #else /* !CONFIG_CAN_CALC_BITTIMING */ > -static int can_calc_bittiming(struct net_device *dev, struct can_bitti= ming *bt) > +static int can_calc_bittiming(struct net_device *dev, struct can_bitti= ming *bt, > + const struct can_bittiming_const *btc) > { > netdev_err(dev, "bit-timing calculation not available\n"); > return -EINVAL; > @@ -217,16 +215,13 @@ static int can_calc_bittiming(struct net_device *= dev, struct can_bittiming *bt) > * prescaler value brp. You can find more information in the header > * file linux/can/netlink.h. > */ > -static int can_fixup_bittiming(struct net_device *dev, struct can_bitt= iming *bt) > +static int can_fixup_bittiming(struct net_device *dev, struct can_bitt= iming *bt, > + const struct can_bittiming_const *btc) > { > struct can_priv *priv =3D netdev_priv(dev); > - const struct can_bittiming_const *btc =3D priv->bittiming_const; > int tseg1, alltseg; > u64 brp64; > =20 > - if (!priv->bittiming_const) > - return -ENOTSUPP; > - > tseg1 =3D bt->prop_seg + bt->phase_seg1; > if (!bt->sjw) > bt->sjw =3D 1; > @@ -254,33 +249,29 @@ static int can_fixup_bittiming(struct net_device = *dev, struct can_bittiming *bt) > return 0; > } > =20 > -static int can_get_bittiming(struct net_device *dev, struct can_bittim= ing *bt) > +static int can_get_bittiming(struct net_device *dev, struct can_bittim= ing *bt, > + const struct can_bittiming_const *btc) > { > - struct can_priv *priv =3D netdev_priv(dev); > - int err; > + int err =3D -EINVAL; Keep err uninitialized and make an explicit else err =3D -EINVAL.... see below: > =20 > /* Check if the CAN device has bit-timing parameters */ > - if (priv->bittiming_const) { > - > - /* Non-expert mode? Check if the bitrate has been pre-defined */ > - if (!bt->tq) { > - /* Determine bit-timing parameters */ > - if (bt->bitrate) > - err =3D can_calc_bittiming(dev, bt); > - else > - return -EINVAL; > - } else { > - /* Check bit-timing params and calculate proper brp */ > - if (!bt->bitrate) > - err =3D can_fixup_bittiming(dev, bt); > - else > - return -EINVAL; > - } > - if (err) > - return err; > + if (!btc) > + return -ENOTSUPP; > + > + if (!bt->tq) { > + /* non-expert mode: Determine bit-timing parameters */ > + if (bt->bitrate) Why not: if (!bt->tq && bt->bitrate) { > + err =3D can_calc_bittiming(dev, bt, btc); > + } else { } else if (bt->tq && !bt->bitrate) { > + /* > + * expert mode: Check bit-timing parameters and > + * calculate proper brp and a human readable bitrate > + */ > + if (!bt->bitrate) > + err =3D can_fixup_bittiming(dev, bt, btc); > } } else { err =3D -EINVAL; } > =20 > - return 0; > + return err; > } > =20 > /* > @@ -674,7 +665,7 @@ static int can_changelink(struct net_device *dev, > if (dev->flags & IFF_UP) > return -EBUSY; > memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt)); > - err =3D can_get_bittiming(dev, &bt); > + err =3D can_get_bittiming(dev, &bt, priv->bittiming_const); > if (err) > return err; > memcpy(&priv->bittiming, &bt, sizeof(bt)); >=20 --=20 Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | --B5AtSnNBA9MMgC687oKId2KxeBrG36mRE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Icedove - http://www.enigmail.net/ iEYEARECAAYFAlMQaRsACgkQjTAFq1RaXHPjVgCeK3JeH+NDCt3jhaeksqI6zce1 GOQAnj3kxV1e4gPuLTMAJcwI/vdWrAN+ =PZox -----END PGP SIGNATURE----- --B5AtSnNBA9MMgC687oKId2KxeBrG36mRE--