Hi Kieran, On Mon, Jan 07, 2019 at 09:49:25AM +0000, Kieran Bingham wrote: > Hi Jacopo > > On 06/01/2019 15:54, Jacopo Mondi wrote: > > Add small is_txb() macro to the existing is_txa() and use it where > > appropriate. > > Thank you. > > I think this will make the code much better to read than if (!is_txa). > > > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > > > > --- > > drivers/media/i2c/adv748x/adv748x-csi2.c | 2 +- > > drivers/media/i2c/adv748x/adv748x.h | 6 +++++- > > 2 files changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/i2c/adv748x/adv748x-csi2.c b/drivers/media/i2c/adv748x/adv748x-csi2.c > > index 6ce21542ed48..b6b5d8c7ea7c 100644 > > --- a/drivers/media/i2c/adv748x/adv748x-csi2.c > > +++ b/drivers/media/i2c/adv748x/adv748x-csi2.c > > @@ -82,7 +82,7 @@ static int adv748x_csi2_registered(struct v4l2_subdev *sd) > > return adv748x_csi2_register_link(tx, sd->v4l2_dev, > > &state->hdmi.sd, > > ADV748X_HDMI_SOURCE); > > - if (!is_txa(tx) && is_afe_enabled(state)) > > + if (is_txb(tx) && is_afe_enabled(state)) > > return adv748x_csi2_register_link(tx, sd->v4l2_dev, > > &state->afe.sd, > > ADV748X_AFE_SOURCE); > > diff --git a/drivers/media/i2c/adv748x/adv748x.h b/drivers/media/i2c/adv748x/adv748x.h > > index b482c7fe6957..bc2da1b5ce29 100644 > > --- a/drivers/media/i2c/adv748x/adv748x.h > > +++ b/drivers/media/i2c/adv748x/adv748x.h > > @@ -89,8 +89,12 @@ struct adv748x_csi2 { > > > > #define notifier_to_csi2(n) container_of(n, struct adv748x_csi2, notifier) > > #define adv748x_sd_to_csi2(sd) container_of(sd, struct adv748x_csi2, sd) > > + > > #define is_tx_enabled(_tx) ((_tx)->state->endpoints[(_tx)->port] != NULL) > > -#define is_txa(_tx) ((_tx) == &(_tx)->state->txa) > > +#define __is_tx(_tx, _ab) ((_tx) == &(_tx)->state->tx##_ab) > > +#define is_txa(_tx) __is_tx(_tx, a) > > +#define is_txb(_tx) __is_tx(_tx, b) > > I would have just duplicated the is_txa() line here... but this is good > too :) I agree it might seem more complex than necessary. I initially made it like this as I started from the 'is_tx()' macro this series adds in 6/6. If it is easier to have an '((_tx) == &(_tx)->state->txb)' I can change this. Thanks j > > > > + > > #define is_afe_enabled(_state) \ > > ((_state)->endpoints[ADV748X_PORT_AIN0] != NULL || \ > > (_state)->endpoints[ADV748X_PORT_AIN1] != NULL || \ > > > > -- > Regards > -- > Kieran
Attachment:
signature.asc
Description: PGP signature