| Message ID | 1524412577-14419-3-git-send-email-akinobu.mita@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mita-san, On Sunday, 22 April 2018 18:56:08 EEST Akinobu Mita wrote: > The ov772x driver only works when the i2c controller have > I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't > support it. > > The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that > it doesn't support repeated starts. > > This changes the reading ov772x register method so that it doesn't > require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages. As commented in a reply to v1, given that this implementation is in no way specific to the ov772x driver, I'd prefer implementing the fallback in the I2C core instead. > Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Hans Verkuil <hans.verkuil@cisco.com> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > * v3 > - Remove I2C_CLIENT_SCCB flag set as it isn't needed anymore > > drivers/media/i2c/ov772x.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > index b62860c..2ae730f 100644 > --- a/drivers/media/i2c/ov772x.c > +++ b/drivers/media/i2c/ov772x.c > @@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev > *sd) return container_of(sd, struct ov772x_priv, subdev); > } > > -static inline int ov772x_read(struct i2c_client *client, u8 addr) > +static int ov772x_read(struct i2c_client *client, u8 addr) > { > - return i2c_smbus_read_byte_data(client, addr); > + int ret; > + u8 val; > + > + ret = i2c_master_send(client, &addr, 1); > + if (ret < 0) > + return ret; > + ret = i2c_master_recv(client, &val, 1); > + if (ret < 0) > + return ret; > + > + return val; > } > > static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 > value) @@ -1255,13 +1265,11 @@ static int ov772x_probe(struct i2c_client > *client, return -EINVAL; > } > > - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > - I2C_FUNC_PROTOCOL_MANGLING)) { > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) { > dev_err(&adapter->dev, > - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or PROTOCOL_MANGLING \n"); > + "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n"); > return -EIO; > } > - client->flags |= I2C_CLIENT_SCCB; > > priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); > if (!priv)
2018-04-23 18:18 GMT+09:00 Laurent Pinchart <laurent.pinchart@ideasonboard.com>: > Hi Mita-san, > > On Sunday, 22 April 2018 18:56:08 EEST Akinobu Mita wrote: >> The ov772x driver only works when the i2c controller have >> I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't >> support it. >> >> The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that >> it doesn't support repeated starts. >> >> This changes the reading ov772x register method so that it doesn't >> require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages. > > As commented in a reply to v1, given that this implementation is in no way > specific to the ov772x driver, I'd prefer implementing the fallback in the I2C > core instead. Do you have any ideas how to implement in the I2C core? I wonder if I can modify i2c_smbus_read_byte_data() or i2c_transfer() for I2C_CLIENT_SCCB. >> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >> Cc: Hans Verkuil <hans.verkuil@cisco.com> >> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> >> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> >> --- >> * v3 >> - Remove I2C_CLIENT_SCCB flag set as it isn't needed anymore >> >> drivers/media/i2c/ov772x.c | 20 ++++++++++++++------ >> 1 file changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c >> index b62860c..2ae730f 100644 >> --- a/drivers/media/i2c/ov772x.c >> +++ b/drivers/media/i2c/ov772x.c >> @@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev >> *sd) return container_of(sd, struct ov772x_priv, subdev); >> } >> >> -static inline int ov772x_read(struct i2c_client *client, u8 addr) >> +static int ov772x_read(struct i2c_client *client, u8 addr) >> { >> - return i2c_smbus_read_byte_data(client, addr); >> + int ret; >> + u8 val; >> + >> + ret = i2c_master_send(client, &addr, 1); >> + if (ret < 0) >> + return ret; >> + ret = i2c_master_recv(client, &val, 1); >> + if (ret < 0) >> + return ret; >> + >> + return val; >> } >> >> static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 >> value) @@ -1255,13 +1265,11 @@ static int ov772x_probe(struct i2c_client >> *client, return -EINVAL; >> } >> >> - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | >> - I2C_FUNC_PROTOCOL_MANGLING)) { >> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) { >> dev_err(&adapter->dev, >> - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or PROTOCOL_MANGLING > \n"); >> + "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n"); >> return -EIO; >> } >> - client->flags |= I2C_CLIENT_SCCB; >> >> priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); >> if (!priv) > > > -- > Regards, > > Laurent Pinchart > > >
Hi Mita-san, (CC'ing Wolfram Sang) On Monday, 23 April 2018 18:55:20 EEST Akinobu Mita wrote: > 2018-04-23 18:18 GMT+09:00 Laurent Pinchart: > > On Sunday, 22 April 2018 18:56:08 EEST Akinobu Mita wrote: > >> The ov772x driver only works when the i2c controller have > >> I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't > >> support it. > >> > >> The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that > >> it doesn't support repeated starts. > >> > >> This changes the reading ov772x register method so that it doesn't > >> require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages. > > > > As commented in a reply to v1, given that this implementation is in no way > > specific to the ov772x driver, I'd prefer implementing the fallback in the > > I2C core instead. > > Do you have any ideas how to implement in the I2C core? > I wonder if I can modify i2c_smbus_read_byte_data() or i2c_transfer() > for I2C_CLIENT_SCCB. How about i2c_smbus_xfer_emulated() ? The tricky part will be to handle the I2C adapters that implement .smbus_xfer(), as those won't go through i2c_smbus_xfer_emulated(). i2c_smbus_xfer_emulated() relies on i2c_transfer(), which itself relies on the I2C adapter's .master_xfer() operation. We're thus only concerned about the drivers that implement both .smbus_xfer() and master_xfer(), and there's only 4 of them (i2c-opal, i2c-pasemi, i2c-powermac and i2c-zx2967). Maybe the simplest solution would be to force the emulation path if I2C_CLIENT_SCCB && !I2C_FUNC_PROTOCOL_MANGLING && ->master_xfer != NULL ? Wolfram, what do you think ? > >> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> > >> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Cc: Hans Verkuil <hans.verkuil@cisco.com> > >> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > >> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> > >> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > >> --- > >> * v3 > >> - Remove I2C_CLIENT_SCCB flag set as it isn't needed anymore > >> > >> drivers/media/i2c/ov772x.c | 20 ++++++++++++++------ > >> 1 file changed, 14 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c > >> index b62860c..2ae730f 100644 > >> --- a/drivers/media/i2c/ov772x.c > >> +++ b/drivers/media/i2c/ov772x.c > >> @@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct > >> v4l2_subdev *sd) > >> return container_of(sd, struct ov772x_priv, subdev); > >> } > >> > >> -static inline int ov772x_read(struct i2c_client *client, u8 addr) > >> +static int ov772x_read(struct i2c_client *client, u8 addr) > >> { > >> - return i2c_smbus_read_byte_data(client, addr); > >> + int ret; > >> + u8 val; > >> + > >> + ret = i2c_master_send(client, &addr, 1); > >> + if (ret < 0) > >> + return ret; > >> + ret = i2c_master_recv(client, &val, 1); > >> + if (ret < 0) > >> + return ret; > >> + > >> + return val; > >> } > >> > >> static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 > >> value) > >> @@ -1255,13 +1265,11 @@ static int ov772x_probe(struct i2c_client > >> *client, > >> return -EINVAL; > >> } > >> > >> - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > >> - I2C_FUNC_PROTOCOL_MANGLING)) > >> { > >> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) { > >> dev_err(&adapter->dev, > >> - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or > >> PROTOCOL_MANGLING\n"); > >> + "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n"); > >> return -EIO; > >> } > >> > >> - client->flags |= I2C_CLIENT_SCCB; > >> > >> priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); > >> if (!priv)
> How about i2c_smbus_xfer_emulated() ? The tricky part will be to handle the > I2C adapters that implement .smbus_xfer(), as those won't go through > i2c_smbus_xfer_emulated(). i2c_smbus_xfer_emulated() relies on i2c_transfer(), > which itself relies on the I2C adapter's .master_xfer() operation. We're thus > only concerned about the drivers that implement both .smbus_xfer() and > master_xfer(), and there's only 4 of them (i2c-opal, i2c-pasemi, i2c-powermac > and i2c-zx2967). Maybe the simplest solution would be to force the emulation > path if I2C_CLIENT_SCCB && !I2C_FUNC_PROTOCOL_MANGLING && ->master_xfer != > NULL ? > > Wolfram, what do you think ? I think it is a mess :) Further: I don't think we will ever see an SMBus controller which allows mangling. SMBus is way more precisely defined than I2C, so HW can then do much more things automatically. They will always do a REP_START, so I don't think you can connect SCCB devices to SMBus. As a result, we shouldn't do SMBus calls for SCCB. Maybe we should introduce sccb_byte_read? SCCB didn't specify much more than byte read IIRC, or? The implementation here with two seperate messages makes much sense to me then. I could argue that the sccb_* helpers should live in drivers/media since it is probably only Omnivision trying to work around I2C licensing here? But if it is not too heavy, maybe we could take it into i2c as well. Makes sense or did I miss something?
Hi Wolfram, On Monday, 23 April 2018 23:11:21 EEST Wolfram Sang wrote: > > How about i2c_smbus_xfer_emulated() ? The tricky part will be to handle > > the I2C adapters that implement .smbus_xfer(), as those won't go through > > i2c_smbus_xfer_emulated(). i2c_smbus_xfer_emulated() relies on > > i2c_transfer(), which itself relies on the I2C adapter's .master_xfer() > > operation. We're thus only concerned about the drivers that implement > > both .smbus_xfer() and master_xfer(), and there's only 4 of them > > (i2c-opal, i2c-pasemi, i2c-powermac and i2c-zx2967). Maybe the simplest > > solution would be to force the emulation path if I2C_CLIENT_SCCB && > > !I2C_FUNC_PROTOCOL_MANGLING && ->master_xfer != NULL ? > > > > Wolfram, what do you think ? > > I think it is a mess :) > > Further: I don't think we will ever see an SMBus controller which allows > mangling. SMBus is way more precisely defined than I2C, so HW can then > do much more things automatically. They will always do a REP_START, so I > don't think you can connect SCCB devices to SMBus. > > As a result, we shouldn't do SMBus calls for SCCB. Maybe we should > introduce sccb_byte_read? SCCB didn't specify much more than byte read > IIRC, or? The implementation here with two seperate messages makes much > sense to me then. > > I could argue that the sccb_* helpers should live in drivers/media since > it is probably only Omnivision trying to work around I2C licensing here? > > But if it is not too heavy, maybe we could take it into i2c as well. > > Makes sense or did I miss something? SCCB helpers would work too. It would be easy to just move the functions defined in this patch to helpers and be done with it. However, there are I2C adapters that have native SCCB support, so to take advantage of that feature we need to forward native SCCB calls all the way down the stack in that case. That's why I thought an implementation in the I2C subsystem would be better. Furthermore, as SCCB is really a slightly mangled version of I2C, I think the I2C subsystem would be a natural location for the implementation. It shouldn't be too much work, it's just a matter of deciding what the call stacks should be for the native vs. emulated cases.
> SCCB helpers would work too. It would be easy to just move the functions > defined in this patch to helpers and be done with it. However, there are I2C > adapters that have native SCCB support, so to take advantage of that feature Ah, didn't notice that so far. Can't find it in drivers/i2c/busses. Where are those? > we need to forward native SCCB calls all the way down the stack in that case. And how is it done currently? > That's why I thought an implementation in the I2C subsystem would be better. > Furthermore, as SCCB is really a slightly mangled version of I2C, I think the > I2C subsystem would be a natural location for the implementation. It shouldn't Can be argued. But it can also be argues that it sits on top of I2C and doesn't need to live in i2c-folders itself (like PMBus). The implementation given in this patch looks a bit like the latter. However, this is not the main question currently. > be too much work, it's just a matter of deciding what the call stacks should > be for the native vs. emulated cases. I don't like it. We shouldn't use SMBus calls for SCCB because SMBus will very likely never support it. Or do you know of such a case? I think I really want sccb helpers. So, people immediately see that SCCB is used and not SMBus or I2C. And there, we can handle native support vs. I2C-SCCB-emulation. And maybe SMBus-SCCB emulation but I doubt we will ever need it.
Hi Wolfram, On Monday, 23 April 2018 23:36:16 EEST Wolfram Sang wrote: > > SCCB helpers would work too. It would be easy to just move the functions > > defined in this patch to helpers and be done with it. However, there are > > I2C adapters that have native SCCB support, so to take advantage of that > > feature > > Ah, didn't notice that so far. Can't find it in drivers/i2c/busses. > Where are those? IIRC the OMAP I2C adapter supports SCCB natively. I'm not sure the driver implements that though. > > we need to forward native SCCB calls all the way down the stack in that > > case. > > And how is it done currently? Currently we go down to .master_xfer(), and adapters can then decide to use the hardware SCCB support. Again, it might not be implemented :-) > > That's why I thought an implementation in the I2C subsystem would be > > better. Furthermore, as SCCB is really a slightly mangled version of I2C, > > I think the I2C subsystem would be a natural location for the > > implementation. It shouldn't > > Can be argued. But it can also be argues that it sits on top of I2C and > doesn't need to live in i2c-folders itself (like PMBus). The > implementation given in this patch looks a bit like the latter. However, > this is not the main question currently. > > > be too much work, it's just a matter of deciding what the call stacks > > should be for the native vs. emulated cases. > > I don't like it. We shouldn't use SMBus calls for SCCB because SMBus > will very likely never support it. Or do you know of such a case? I > think I really want sccb helpers. So, people immediately see that SCCB > is used and not SMBus or I2C. And there, we can handle native support > vs. I2C-SCCB-emulation. And maybe SMBus-SCCB emulation but I doubt we > will ever need it. I'm fine with SCCB helpers. Please note, however, that SCCB differs from SMBus in two ways: NACKs shall be ignored by the master (even though most SCCB devices generate an ack, so we could likely ignore this), and write-read sequences shouldn't use a repeated start. Apart from that register reads and register writes are identical to SMBus, which prompted the reuse (or abuse) of the SMBus API. If we end up implementing SCCB helpers, they will likely look very, very similar to the SMBus implementation, including the SMBus emulated transfer helper.
Hi guys, On Mon, Apr 23, 2018 at 11:51:30PM +0300, Laurent Pinchart wrote: > Hi Wolfram, > > On Monday, 23 April 2018 23:36:16 EEST Wolfram Sang wrote: > > > SCCB helpers would work too. It would be easy to just move the functions > > > defined in this patch to helpers and be done with it. However, there are > > > I2C adapters that have native SCCB support, so to take advantage of that > > > feature > > > > Ah, didn't notice that so far. Can't find it in drivers/i2c/busses. > > Where are those? > > IIRC the OMAP I2C adapter supports SCCB natively. I'm not sure the driver > implements that though. > > > > we need to forward native SCCB calls all the way down the stack in that > > > case. > > > > And how is it done currently? > > Currently we go down to .master_xfer(), and adapters can then decide to use > the hardware SCCB support. Again, it might not be implemented :-) > > > > That's why I thought an implementation in the I2C subsystem would be > > > better. Furthermore, as SCCB is really a slightly mangled version of I2C, > > > I think the I2C subsystem would be a natural location for the > > > implementation. It shouldn't > > > > Can be argued. But it can also be argues that it sits on top of I2C and > > doesn't need to live in i2c-folders itself (like PMBus). The > > implementation given in this patch looks a bit like the latter. However, > > this is not the main question currently. > > > > > be too much work, it's just a matter of deciding what the call stacks > > > should be for the native vs. emulated cases. > > > > I don't like it. We shouldn't use SMBus calls for SCCB because SMBus > > will very likely never support it. Or do you know of such a case? I > > think I really want sccb helpers. So, people immediately see that SCCB > > is used and not SMBus or I2C. And there, we can handle native support > > vs. I2C-SCCB-emulation. And maybe SMBus-SCCB emulation but I doubt we > > will ever need it. > > I'm fine with SCCB helpers. Please note, however, that SCCB differs from SMBus > in two ways: NACKs shall be ignored by the master (even though most SCCB > devices generate an ack, so we could likely ignore this), and write-read > sequences shouldn't use a repeated start. Apart from that register reads and > register writes are identical to SMBus, which prompted the reuse (or abuse) of > the SMBus API. If we end up implementing SCCB helpers, they will likely look > very, very similar to the SMBus implementation, including the SMBus emulated > transfer helper. Sounds like that there would be a bit of work to do here but AFAIU the patchset should be fine to go in (with the other comments addressed) and this could be addressed separately. Let me know if you have concerns. Thanks.
> > Ah, didn't notice that so far. Can't find it in drivers/i2c/busses. > > Where are those? > > IIRC the OMAP I2C adapter supports SCCB natively. I'm not sure the driver > implements that though. It doesn't currently. And seeing how long it sits in HW without a driver for it, I don't have much expectations. > > > we need to forward native SCCB calls all the way down the stack in that > > > case. > > > > And how is it done currently? > > Currently we go down to .master_xfer(), and adapters can then decide to use > the hardware SCCB support. Again, it might not be implemented :-) To sum it up: hardware-driven SCCB support is a very rare exception not implemented anywhere in all those years. From a pragmatic point of view, I'd say: we should be open for it, but we don't need to design around it. > I'm fine with SCCB helpers. Please note, however, that SCCB differs from SMBus > in two ways: NACKs shall be ignored by the master (even though most SCCB > devices generate an ack, so we could likely ignore this), and write-read > sequences shouldn't use a repeated start. Apart from that register reads and Especially the latter is a huge difference to SMBus, and so I think it will be much safer to not abuse SMBus calls for SCCB. > register writes are identical to SMBus, which prompted the reuse (or abuse) of > the SMBus API. If we end up implementing SCCB helpers, they will likely look > very, very similar to the SMBus implementation, including the SMBus emulated > transfer helper. I don't think so. SCCB has much less transaction types than SMBus. Also, the fallback as sketched in this patch (one master write, then a master read) would be impossible on SMBus. I have an idea in my mind. Maybe it is better to implement an RFC first, so we can talk over code (even if only in prototype stage). I already found this in ov7670.c, so I am proven wrong on this one already: 472 * Note that there are two versions of these. On the XO 1, the 473 * i2c controller only does SMBUS, so that's what we use. The 474 * ov7670 is not really an SMBUS device, though, so the communication 475 * is not always entirely reliable. Sigh...
diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c index b62860c..2ae730f 100644 --- a/drivers/media/i2c/ov772x.c +++ b/drivers/media/i2c/ov772x.c @@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev *sd) return container_of(sd, struct ov772x_priv, subdev); } -static inline int ov772x_read(struct i2c_client *client, u8 addr) +static int ov772x_read(struct i2c_client *client, u8 addr) { - return i2c_smbus_read_byte_data(client, addr); + int ret; + u8 val; + + ret = i2c_master_send(client, &addr, 1); + if (ret < 0) + return ret; + ret = i2c_master_recv(client, &val, 1); + if (ret < 0) + return ret; + + return val; } static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value) @@ -1255,13 +1265,11 @@ static int ov772x_probe(struct i2c_client *client, return -EINVAL; } - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | - I2C_FUNC_PROTOCOL_MANGLING)) { + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) { dev_err(&adapter->dev, - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or PROTOCOL_MANGLING\n"); + "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n"); return -EIO; } - client->flags |= I2C_CLIENT_SCCB; priv = devm_kzalloc(&client->dev, sizeof(*priv), GFP_KERNEL); if (!priv)
The ov772x driver only works when the i2c controller have I2C_FUNC_PROTOCOL_MANGLING. However, many i2c controller drivers don't support it. The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that it doesn't support repeated starts. This changes the reading ov772x register method so that it doesn't require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages. Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Hans Verkuil <hans.verkuil@cisco.com> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- * v3 - Remove I2C_CLIENT_SCCB flag set as it isn't needed anymore drivers/media/i2c/ov772x.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)