Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1258614 > unrolled thread

Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 kbuild test robot <lkp@intel.com> - 2015-10-29 08:30 +0100
    Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Michael Welling <mwelling@ieee.org> - 2015-10-29 15:30 +0100
      Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Arnd Bergmann <arnd@arndb.de> - 2015-10-29 22:50 +0100
        Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Michael Welling <mwelling@ieee.org> - 2015-10-29 23:00 +0100
          Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2015-10-29 23:30 +0100
            Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Mark Brown <broonie@kernel.org> - 2015-10-30 01:40 +0100
              Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Michael Welling <mwelling@ieee.org> - 2015-10-30 02:10 +0100
                Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004 Arnd Bergmann <arnd@arndb.de> - 2015-10-30 11:50 +0100

#1258614 — Re: [PATCH v3] Input: tsc2005 - Add support for tsc2004

[Multipart message — attachments visible in raw view] — view raw

Hi Michael,

[auto build test ERROR on input/next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Michael-Welling/Input-tsc2005-Add-support-for-tsc2004/20151029-081504
config: x86_64-randconfig-s1-10291412 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `tsc2005_cmd':
>> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [next] | [standalone]

#1258825

On Thu, Oct 29, 2015 at 03:25:34PM +0800, kbuild test robot wrote:
> Hi Michael,
> 
> [auto build test ERROR on input/next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
> 
> url:    https://github.com/0day-ci/linux/commits/Michael-Welling/Input-tsc2005-Add-support-for-tsc2004/20151029-081504
> config: x86_64-randconfig-s1-10291412 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/built-in.o: In function `tsc2005_cmd':
> >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'

Argh!

How do I fix this one?

> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259032

On Thursday 29 October 2015 09:22:37 Michael Welling wrote:
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    drivers/built-in.o: In function `tsc2005_cmd':
> > >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'
> 
> Argh!
> 
> How do I fix this one?
> 

Move all the I2C specific code into the tsc2004.c file and remove the
#ifdef. The problem is that tsc200x-core.c is used as built-in when
TOUCHSCREEN_TSC2005=y and I2C=m, so it can't call functions that are
defined in i2c-core.c.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259033

On Thu, Oct 29, 2015 at 10:46:20PM +0100, Arnd Bergmann wrote:
> On Thursday 29 October 2015 09:22:37 Michael Welling wrote:
> > > 
> > > All errors (new ones prefixed by >>):
> > > 
> > >    drivers/built-in.o: In function `tsc2005_cmd':
> > > >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'
> > 
> > Argh!
> > 
> > How do I fix this one?
> > 
> 
> Move all the I2C specific code into the tsc2004.c file and remove the
> #ifdef. The problem is that tsc200x-core.c is used as built-in when
> TOUCHSCREEN_TSC2005=y and I2C=m, so it can't call functions that are
> defined in i2c-core.c.

How would I call the function from the core then?

-Michael
> 
> 	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259043

On Thu, Oct 29, 2015 at 04:53:53PM -0500, Michael Welling wrote:
> On Thu, Oct 29, 2015 at 10:46:20PM +0100, Arnd Bergmann wrote:
> > On Thursday 29 October 2015 09:22:37 Michael Welling wrote:
> > > > 
> > > > All errors (new ones prefixed by >>):
> > > > 
> > > >    drivers/built-in.o: In function `tsc2005_cmd':
> > > > >> tsc200x-core.c:(.text+0x2ae07f): undefined reference to `i2c_smbus_write_byte'
> > > 
> > > Argh!
> > > 
> > > How do I fix this one?
> > > 
> > 
> > Move all the I2C specific code into the tsc2004.c file and remove the
> > #ifdef. The problem is that tsc200x-core.c is used as built-in when
> > TOUCHSCREEN_TSC2005=y and I2C=m, so it can't call functions that are
> > defined in i2c-core.c.
> 
> How would I call the function from the core then?

You can pass a cmd function from the transport module into core module
and store and use it.

However, you have regmap in the driver core already. Mark, is it
possible to have regmap API also allow doing raw underlying protocol
transfer so that consumers could issue command requests without needing
to know if they need to do it over i2c or spi or whatever. Or we need a
notion of command registers in regmap...

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259137

[Multipart message — attachments visible in raw view] — view raw

On Thu, Oct 29, 2015 at 03:23:31PM -0700, Dmitry Torokhov wrote:

> However, you have regmap in the driver core already. Mark, is it
> possible to have regmap API also allow doing raw underlying protocol
> transfer so that consumers could issue command requests without needing
> to know if they need to do it over i2c or spi or whatever. Or we need a
> notion of command registers in regmap...

I don't think it's a good idea to break the encapsulation of the regmap
and export the raw I/O functionality directly, there seem to be more bad
ways of using that than good.  The driver must at some point know what
bus it is dealing with and be able to manage this itself.

I don't know what "command registers" are.

[toc] | [prev] | [next] | [standalone]

#1259145

On Fri, Oct 30, 2015 at 09:39:05AM +0900, Mark Brown wrote:
> On Thu, Oct 29, 2015 at 03:23:31PM -0700, Dmitry Torokhov wrote:
> 
> > However, you have regmap in the driver core already. Mark, is it
> > possible to have regmap API also allow doing raw underlying protocol
> > transfer so that consumers could issue command requests without needing
> > to know if they need to do it over i2c or spi or whatever. Or we need a
> > notion of command registers in regmap...
> 
> I don't think it's a good idea to break the encapsulation of the regmap
> and export the raw I/O functionality directly, there seem to be more bad
> ways of using that than good.  The driver must at some point know what
> bus it is dealing with and be able to manage this itself.
> 
> I don't know what "command registers" are.

With this device, if the MSB of the first byte of the transaction is 1 then
a convertor command is encoded in that byte instead of a register address.

So here is my plan:
- Add a function pointer for tsc2005_cmd in the struct tsc2005
- Put the spi and i2c tsc2005_cmd versions in their respective drivers
- Pass the cmd functions to the core via tsc200x_probe

Any objections?
Other review suggestions before I code the revision?
Am I doing too much with a single patch?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259388

On Thursday 29 October 2015 20:01:51 Michael Welling wrote:
> On Fri, Oct 30, 2015 at 09:39:05AM +0900, Mark Brown wrote:
> > On Thu, Oct 29, 2015 at 03:23:31PM -0700, Dmitry Torokhov wrote:
> > 
> > > However, you have regmap in the driver core already. Mark, is it
> > > possible to have regmap API also allow doing raw underlying protocol
> > > transfer so that consumers could issue command requests without needing
> > > to know if they need to do it over i2c or spi or whatever. Or we need a
> > > notion of command registers in regmap...
> > 
> > I don't think it's a good idea to break the encapsulation of the regmap
> > and export the raw I/O functionality directly, there seem to be more bad
> > ways of using that than good.  The driver must at some point know what
> > bus it is dealing with and be able to manage this itself.
> > 
> > I don't know what "command registers" are.
> 
> With this device, if the MSB of the first byte of the transaction is 1 then
> a convertor command is encoded in that byte instead of a register address.
> 
> So here is my plan:
> - Add a function pointer for tsc2005_cmd in the struct tsc2005
> - Put the spi and i2c tsc2005_cmd versions in their respective drivers
> - Pass the cmd functions to the core via tsc200x_probe
> 
> Any objections?

Sounds good. If you end up needing more than one function pointer,
better pass a 'const' pointer to a structure of function pointers that
you can declare statically in the driver.

> Other review suggestions before I code the revision?
> Am I doing too much with a single patch?

Yes.

Please split out the DT binding changes into a separate patch, and
start with one patch that just moves the common parts out of the SPI
driver before you add the new driver.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web