| Message ID | 20190703214512.41319-1-jeffrey.l.hugo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ti-sn65dsi86 DSI configuration support | expand |
On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: > Add basic support with a simple implementation that utilizes the generic > read/write commands to allow device registers to be configured. This looks good to me but I really don't know anything about DSI, I'd appreciate some review from other people who do. I take it there's some spec thing in DSI that says registers and bytes must both be 8 bit? A couple of minor comments, no need to resend just for these: > + payload[0] = (char)reg; > + payload[1] = (char)val; Do you need the casts? > + ret = mipi_dsi_generic_write(dsi, payload, 2); > + return ret < 0 ? ret : 0; Please just write an if statement, it helps with legibility. > +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, > + const struct regmap_config *config, > + struct lock_class_key *lock_key, > + const char *lock_name) > +{ > + return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, > + lock_key, lock_name); > +} > +EXPORT_SYMBOL_GPL(__regmap_init_dsi); Perhaps validate that the config is OK (mainly the register/value sizes)? Though I'm not sure it's worth it so perhaps not - up to you.
On Fri, Jul 5, 2019 at 7:06 PM Mark Brown <broonie@kernel.org> wrote: > > On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: > > Add basic support with a simple implementation that utilizes the generic > > read/write commands to allow device registers to be configured. > > This looks good to me but I really don't know anything about DSI, > I'd appreciate some review from other people who do. I take it > there's some spec thing in DSI that says registers and bytes must > both be 8 bit? DSI appears to reside under DRM, and the DRM maintainers are copied on this thread, so hopefully they will chime in. Context on DSI: The MIPI (Mobile Industry Processor Interface) Alliance DSI (Display Serial Interface) spec defines an interface between host processors and displays for embedded applications (smartphones and the like). The spec itself is private to MIPI members, although I suspect if you run some queries on your preferred search engine, you may find some accessible copies of it floating around somewhere. The spec defines some specific messages that run over the DSI link. Most of those are grouped into the purposes of sending pixel data over to the display, or configuring gamma, etc. As far as I can tell, DSI does not require these operations be backed by registers, however the several implementations I've seen do it that way. The spec does mandate that to configure something like gamma, one needs to send a message with a specific address, and payload. The addresses for these spec defined messages are 8-bit wide, so 256 valid "destinations". However, the payload is variable. Most of the defined operations take an 8-bit payload, but there are a few that I see with 16-bit payloads. The DSI spec defines two mechanisms for implementation specific configuration (what I'm attempting to do with this series). You can use a spec defined message to select a different set of registers (called a different page), after which point, the addresses of the messages target implementation specific functionality. I've seen this used a lot on the specific panels which can be directly connected to DSI. The second mechanism is to use the generic read/write messages, which the spec says are implementation defined - essentially the spec defines the message type but the contents of the message are not spec defined. This is the mechanism the TI bridge uses. As the contents of the generic read/write messages are implementation defined, the answer to your question seems to be no - the spec does not define that the registers are 8-bit addressable, and 8-bit wide. In running this series more, I actually found a bug with it. It turns out that the TI bridge requires 16-bit addressing (LSB ordering), with the upper 8-bit reserved for future use, but only on reads. Writes are 8-bit addressing. This is part of that implementation specific details. I think perhaps the discussion needs to step back a bit, and decide how flexible do we want this regmap over DSI to be? I think its usefulness comes from when a device can be configured via multiple interfaces, so I don't expect it to be useful for every DSI interface. It seems like the DSI panels use DSI directly to craft their configuration. As a result, we are probably looking at just devices which use the generic read/write commands, but sadly the format for those is not universal per the spec. From the implementations I've seen, I suspect 8-bit addressing of 8-bit wide registers to be the most common, but apparently there is an exception to that already in the one device that I care about. Do we want to go forward with this regmap support just for the one TI device, and see what other usecases come out of it, and attempt to solve those as we go? > > A couple of minor comments, no need to resend just for these: > > > + payload[0] = (char)reg; > > + payload[1] = (char)val; > > Do you need the casts? Apparently not. I was assuming the compiler would complain about implicit truncation. > > > + ret = mipi_dsi_generic_write(dsi, payload, 2); > > + return ret < 0 ? ret : 0; > > Please just write an if statement, it helps with legibility. Uhh, sure. There appear to be several instances of the trinary operator in drivers/base/regmap/ but if an explicit if statement here makes you happy, then I'll do it. > > > +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, > > + const struct regmap_config *config, > > + struct lock_class_key *lock_key, > > + const char *lock_name) > > +{ > > + return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, > > + lock_key, lock_name); > > +} > > +EXPORT_SYMBOL_GPL(__regmap_init_dsi); > > Perhaps validate that the config is OK (mainly the register/value > sizes)? Though I'm not sure it's worth it so perhaps not - up to > you. Probably. Based on the above discussion, should I be making use of reg_read/reg_write in the config?
On 06.07.2019 03:06, Mark Brown wrote: > On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: >> Add basic support with a simple implementation that utilizes the generic >> read/write commands to allow device registers to be configured. > This looks good to me but I really don't know anything about DSI, > I'd appreciate some review from other people who do. I take it > there's some spec thing in DSI that says registers and bytes must > both be 8 bit? I am little bit confused about regmap usage here. On the one hand it nicely fits to this specific driver, probably because it already uses regmap_i2c. On the other it will be unusable for almost all current DSI drivers and probably for most new drivers. Why? 1. DSI protocol defines actually more than 30 types of transactions[1], but this patchset implements only few of them (dsi generic write/read family). Is it possible to implement multiple types of transactions in regmap? 2. There is already some set of helpers which uses dsi bus, rewriting it on regmap is possible or driver could use of regmap and direct access together, the question is if it is really necessary. 3. DSI devices are no MFDs so regmap abstraction has no big value added (correct me, if there are other significant benefits). [1]: https://elixir.bootlin.com/linux/latest/source/include/video/mipi_display.h#L15 Regards Andrzej > > A couple of minor comments, no need to resend just for these: > >> + payload[0] = (char)reg; >> + payload[1] = (char)val; > Do you need the casts? > >> + ret = mipi_dsi_generic_write(dsi, payload, 2); >> + return ret < 0 ? ret : 0; > Please just write an if statement, it helps with legibility. > >> +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, >> + const struct regmap_config *config, >> + struct lock_class_key *lock_key, >> + const char *lock_name) >> +{ >> + return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, >> + lock_key, lock_name); >> +} >> +EXPORT_SYMBOL_GPL(__regmap_init_dsi); > Perhaps validate that the config is OK (mainly the register/value > sizes)? Though I'm not sure it's worth it so perhaps not - up to > you.
On Thu, Jul 11, 2019 at 6:11 AM Andrzej Hajda <a.hajda@samsung.com> wrote: > > On 06.07.2019 03:06, Mark Brown wrote: > > On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: > >> Add basic support with a simple implementation that utilizes the generic > >> read/write commands to allow device registers to be configured. > > This looks good to me but I really don't know anything about DSI, > > I'd appreciate some review from other people who do. I take it > > there's some spec thing in DSI that says registers and bytes must > > both be 8 bit? > > > I am little bit confused about regmap usage here. On the one hand it > nicely fits to this specific driver, probably because it already uses > regmap_i2c. > > On the other it will be unusable for almost all current DSI drivers and > probably for most new drivers. Why? > > 1. DSI protocol defines actually more than 30 types of transactions[1], > but this patchset implements only few of them (dsi generic write/read > family). Is it possible to implement multiple types of transactions in > regmap? > > 2. There is already some set of helpers which uses dsi bus, rewriting it > on regmap is possible or driver could use of regmap and direct access > together, the question is if it is really necessary. > > 3. DSI devices are no MFDs so regmap abstraction has no big value added > (correct me, if there are other significant benefits). > I assume it is not *just* this one bridge that can be programmed over either i2c or dsi, depending on how things are wired up on the board. It certainly would be nice for regmap to support this case, so we don't have to write two different bridge drivers for the same bridge. I wouldn't expect a panel that is only programmed via dsi to use this. BR, -R > > [1]: > https://elixir.bootlin.com/linux/latest/source/include/video/mipi_display.h#L15 > > > Regards > > Andrzej > > > > > > A couple of minor comments, no need to resend just for these: > > > >> + payload[0] = (char)reg; > >> + payload[1] = (char)val; > > Do you need the casts? > > > >> + ret = mipi_dsi_generic_write(dsi, payload, 2); > >> + return ret < 0 ? ret : 0; > > Please just write an if statement, it helps with legibility. > > > >> +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, > >> + const struct regmap_config *config, > >> + struct lock_class_key *lock_key, > >> + const char *lock_name) > >> +{ > >> + return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, > >> + lock_key, lock_name); > >> +} > >> +EXPORT_SYMBOL_GPL(__regmap_init_dsi); > > Perhaps validate that the config is OK (mainly the register/value > > sizes)? Though I'm not sure it's worth it so perhaps not - up to > > you. > >
On Wed, Jul 10, 2019 at 12:08:34PM -0600, Jeffrey Hugo wrote: > On Fri, Jul 5, 2019 at 7:06 PM Mark Brown <broonie@kernel.org> wrote: > The addresses for these spec defined messages are 8-bit wide, so 256 > valid "destinations". However, the payload is variable. Most of the > defined operations take an 8-bit payload, but there are a few that I > see with 16-bit payloads. Oh, good, variable register sizes, what a market leading idea :( That basically doesn't work with regmap, you need to either define one regmap per register size and attach them to the device or use reg_read() and reg_write() and hide the complexity in there. > As the contents of the generic read/write messages are implementation > defined, the answer to your question seems to be no - the spec does > not define that the registers are 8-bit addressable, and 8-bit wide. The code definitely ought to at least be more flexible then. Right now it's very hard coded. > I think perhaps the discussion needs to step back a bit, and decide > how flexible do we want this regmap over DSI to be? I think its > usefulness comes from when a device can be configured via multiple > interfaces, so I don't expect it to be useful for every DSI interface. > It seems like the DSI panels use DSI directly to craft their > configuration. As a result, we are probably looking at just devices > which use the generic read/write commands, but sadly the format for > those is not universal per the spec. From the implementations I've > seen, I suspect 8-bit addressing of 8-bit wide registers to be the > most common, but apparently there is an exception to that already in > the one device that I care about. It's relatively easy to add a bunch of special cases in - look at how the I2C code handles it, keying off a combination of the register configuration and the capabilities of the host controller. I guess for this it'd mainly be the register configuration. You might find the reg_read()/reg_write() interface better than the raw buffer one for some of the formats, it does let > Do we want to go forward with this regmap support just for the one TI > device, and see what other usecases come out of it, and attempt to > solve those as we go? I have no strong opinions here, it looks fine from a framework point of view though it's unclear to me if viewing it as a register map meshes well with how the hardware is designed or not - it seems plausible though.
On Thu, Jul 11, 2019 at 03:11:56PM +0200, Andrzej Hajda wrote: > 1. DSI protocol defines actually more than 30 types of transactions[1], > but this patchset implements only few of them (dsi generic write/read > family). Is it possible to implement multiple types of transactions in > regmap? You can, there's a couple of different ways depending on how exactly things are done. > 3. DSI devices are no MFDs so regmap abstraction has no big value added > (correct me, if there are other significant benefits). There's a few extra bits even if you're not using the marshalling code to get things onto the bus - the main ones are the register cache support (which people often use for simpler suspend/resume support) and the debug and trace facilities (things like tracepoints and debugfs for dumping the register map). There's no real connection to MFDs, I'd say the majority of users are not MFDs.
On 11.07.2019 15:56, Rob Clark wrote: > On Thu, Jul 11, 2019 at 6:11 AM Andrzej Hajda <a.hajda@samsung.com> wrote: >> On 06.07.2019 03:06, Mark Brown wrote: >>> On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: >>>> Add basic support with a simple implementation that utilizes the generic >>>> read/write commands to allow device registers to be configured. >>> This looks good to me but I really don't know anything about DSI, >>> I'd appreciate some review from other people who do. I take it >>> there's some spec thing in DSI that says registers and bytes must >>> both be 8 bit? >> >> I am little bit confused about regmap usage here. On the one hand it >> nicely fits to this specific driver, probably because it already uses >> regmap_i2c. >> >> On the other it will be unusable for almost all current DSI drivers and >> probably for most new drivers. Why? >> >> 1. DSI protocol defines actually more than 30 types of transactions[1], >> but this patchset implements only few of them (dsi generic write/read >> family). Is it possible to implement multiple types of transactions in >> regmap? >> >> 2. There is already some set of helpers which uses dsi bus, rewriting it >> on regmap is possible or driver could use of regmap and direct access >> together, the question is if it is really necessary. >> >> 3. DSI devices are no MFDs so regmap abstraction has no big value added >> (correct me, if there are other significant benefits). >> > I assume it is not *just* this one bridge that can be programmed over > either i2c or dsi, depending on how things are wired up on the board. > It certainly would be nice for regmap to support this case, so we > don't have to write two different bridge drivers for the same bridge. > I wouldn't expect a panel that is only programmed via dsi to use this. On the other side supporting DSI and I2C in one driver is simply matter of writing proper accesors. Regards Andrzej > > BR, > -R > >> [1]: >> https://elixir.bootlin.com/linux/latest/source/include/video/mipi_display.h#L15 >> >> >> Regards >> >> Andrzej >> >> >>> A couple of minor comments, no need to resend just for these: >>> >>>> + payload[0] = (char)reg; >>>> + payload[1] = (char)val; >>> Do you need the casts? >>> >>>> + ret = mipi_dsi_generic_write(dsi, payload, 2); >>>> + return ret < 0 ? ret : 0; >>> Please just write an if statement, it helps with legibility. >>> >>>> +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, >>>> + const struct regmap_config *config, >>>> + struct lock_class_key *lock_key, >>>> + const char *lock_name) >>>> +{ >>>> + return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, >>>> + lock_key, lock_name); >>>> +} >>>> +EXPORT_SYMBOL_GPL(__regmap_init_dsi); >>> Perhaps validate that the config is OK (mainly the register/value >>> sizes)? Though I'm not sure it's worth it so perhaps not - up to >>> you. >>
On Fri, Jul 12, 2019 at 7:01 AM Andrzej Hajda <a.hajda@samsung.com> wrote: > > On 11.07.2019 15:56, Rob Clark wrote: > > On Thu, Jul 11, 2019 at 6:11 AM Andrzej Hajda <a.hajda@samsung.com> wrote: > >> On 06.07.2019 03:06, Mark Brown wrote: > >>> On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: > >>>> Add basic support with a simple implementation that utilizes the generic > >>>> read/write commands to allow device registers to be configured. > >>> This looks good to me but I really don't know anything about DSI, > >>> I'd appreciate some review from other people who do. I take it > >>> there's some spec thing in DSI that says registers and bytes must > >>> both be 8 bit? > >> > >> I am little bit confused about regmap usage here. On the one hand it > >> nicely fits to this specific driver, probably because it already uses > >> regmap_i2c. > >> > >> On the other it will be unusable for almost all current DSI drivers and > >> probably for most new drivers. Why? > >> > >> 1. DSI protocol defines actually more than 30 types of transactions[1], > >> but this patchset implements only few of them (dsi generic write/read > >> family). Is it possible to implement multiple types of transactions in > >> regmap? > >> > >> 2. There is already some set of helpers which uses dsi bus, rewriting it > >> on regmap is possible or driver could use of regmap and direct access > >> together, the question is if it is really necessary. > >> > >> 3. DSI devices are no MFDs so regmap abstraction has no big value added > >> (correct me, if there are other significant benefits). > >> > > I assume it is not *just* this one bridge that can be programmed over > > either i2c or dsi, depending on how things are wired up on the board. > > It certainly would be nice for regmap to support this case, so we > > don't have to write two different bridge drivers for the same bridge. > > I wouldn't expect a panel that is only programmed via dsi to use this. > > > On the other side supporting DSI and I2C in one driver is simply matter > of writing proper accesors. To me, this reads like your counter argument to not using regmap, is to reinvent regmap. Maybe I don't understand what you are proposing here, but it sounds like remove the regmap support, define sn65 specific accessors that just before sending the write to the bus does a check if the access needs to go over i2c or DSI. Feels like a clunky version of regmap to me. Why not use the existing "generic" framework? To your point that DSI defines over 30 message types, yes it does, but that seems to be outside of the scope. How many of those are actually for doing register access? I'm thinking just 4 (technically a hair more than that because of the multiple version of the same message) - generic read, generic write, dcs read, dcs write. I don't view regmap as a generic abstraction layer over a particular mechanism, and thus needs to support everything that mechanism does. Sending sync commands, or pixel data over DSI is outside the scope of regmap to me.
On Fri, Jul 12, 2019 at 7:22 AM Jeffrey Hugo <jeffrey.l.hugo@gmail.com> wrote: > > On Fri, Jul 12, 2019 at 7:01 AM Andrzej Hajda <a.hajda@samsung.com> wrote: > > > > On 11.07.2019 15:56, Rob Clark wrote: > > > On Thu, Jul 11, 2019 at 6:11 AM Andrzej Hajda <a.hajda@samsung.com> wrote: > > >> On 06.07.2019 03:06, Mark Brown wrote: > > >>> On Wed, Jul 03, 2019 at 02:45:12PM -0700, Jeffrey Hugo wrote: > > >>>> Add basic support with a simple implementation that utilizes the generic > > >>>> read/write commands to allow device registers to be configured. > > >>> This looks good to me but I really don't know anything about DSI, > > >>> I'd appreciate some review from other people who do. I take it > > >>> there's some spec thing in DSI that says registers and bytes must > > >>> both be 8 bit? > > >> > > >> I am little bit confused about regmap usage here. On the one hand it > > >> nicely fits to this specific driver, probably because it already uses > > >> regmap_i2c. > > >> > > >> On the other it will be unusable for almost all current DSI drivers and > > >> probably for most new drivers. Why? > > >> > > >> 1. DSI protocol defines actually more than 30 types of transactions[1], > > >> but this patchset implements only few of them (dsi generic write/read > > >> family). Is it possible to implement multiple types of transactions in > > >> regmap? > > >> > > >> 2. There is already some set of helpers which uses dsi bus, rewriting it > > >> on regmap is possible or driver could use of regmap and direct access > > >> together, the question is if it is really necessary. > > >> > > >> 3. DSI devices are no MFDs so regmap abstraction has no big value added > > >> (correct me, if there are other significant benefits). > > >> > > > I assume it is not *just* this one bridge that can be programmed over > > > either i2c or dsi, depending on how things are wired up on the board. > > > It certainly would be nice for regmap to support this case, so we > > > don't have to write two different bridge drivers for the same bridge. > > > I wouldn't expect a panel that is only programmed via dsi to use this. > > > > > > On the other side supporting DSI and I2C in one driver is simply matter > > of writing proper accesors. > > To me, this reads like your counter argument to not using regmap, is > to reinvent regmap. Maybe I don't understand what you are proposing > here, but it sounds like remove the regmap support, define sn65 > specific accessors that just before sending the write to the bus does > a check if the access needs to go over i2c or DSI. Feels like a > clunky version of regmap to me. Why not use the existing "generic" > framework? > > To your point that DSI defines over 30 message types, yes it does, but > that seems to be outside of the scope. How many of those are actually > for doing register access? I'm thinking just 4 (technically a hair > more than that because of the multiple version of the same message) - > generic read, generic write, dcs read, dcs write. I don't view regmap > as a generic abstraction layer over a particular mechanism, and thus > needs to support everything that mechanism does. Sending sync > commands, or pixel data over DSI is outside the scope of regmap to me. I'm w/ jhugo on this one.. if you are working w/ a device that can be programmed via i2c or dsi, regmap and limiting yourself to the small subset of dsi cmds which map to i2c reads/writes, makes a ton of sense. (And I'm almost certain this bridge isn't the only such device.) That isn't to say you should use regmap for devices that are only programmed over dsi. That would be silly. The original argument about this not being usable by most DSI devices is really a non-sequitur. BR, -R
On 11.07.2019 16:50, Mark Brown wrote: > On Thu, Jul 11, 2019 at 03:11:56PM +0200, Andrzej Hajda wrote: > >> 1. DSI protocol defines actually more than 30 types of transactions[1], >> but this patchset implements only few of them (dsi generic write/read >> family). Is it possible to implement multiple types of transactions in >> regmap? > You can, there's a couple of different ways depending on how > exactly things are done. > >> 3. DSI devices are no MFDs so regmap abstraction has no big value added >> (correct me, if there are other significant benefits). > There's a few extra bits even if you're not using the marshalling > code to get things onto the bus - the main ones are the register > cache support (which people often use for simpler suspend/resume > support) and the debug and trace facilities (things like > tracepoints and debugfs for dumping the register map). I do not see cache usable in bridge drivers, I guess default config will be caching disabled, as it is already in the driver from this patchset. So beside marshaling, we are left only with debug facilities, not a big gain :) Moreover as it was already written DSI is mainly used to transport COMMANDS to the device, with variable number of arguments - it does not resembles registry map at all. On the other side there is some subset of DSI devices which exposes register memory using MIPI DSI Generic Write/Read packets, for example: ti-sn65dsi86, tc358764. They fit better to regmap framework. Hard to say how common is this pattern. Maybe we can try with it? If yes it would be good to put clear remark that regmap/dsi is for such devices, to avoid possible confusion. Regards Andrzej
diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig index c8bbf5322720..27669afa9d95 100644 --- a/drivers/base/regmap/Kconfig +++ b/drivers/base/regmap/Kconfig @@ -4,7 +4,7 @@ # subsystems should select the appropriate symbols. config REGMAP - default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_I3C) + default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_I3C || REGMAP_DSI) select IRQ_DOMAIN if REGMAP_IRQ bool @@ -53,3 +53,7 @@ config REGMAP_SCCB config REGMAP_I3C tristate depends on I3C + +config REGMAP_DSI + tristate + depends on DRM_MIPI_DSI diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile index ff6c7d8ec1cd..c1cc81f3986f 100644 --- a/drivers/base/regmap/Makefile +++ b/drivers/base/regmap/Makefile @@ -17,3 +17,4 @@ obj-$(CONFIG_REGMAP_W1) += regmap-w1.o obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o +obj-$(CONFIG_REGMAP_DSI) += regmap-dsi.o diff --git a/drivers/base/regmap/regmap-dsi.c b/drivers/base/regmap/regmap-dsi.c new file mode 100644 index 000000000000..0c2900e2fee0 --- /dev/null +++ b/drivers/base/regmap/regmap-dsi.c @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Register map access API - DSI support +// +// Copyright (c) 2019 Jeffrey Hugo + +#include <drm/drm_mipi_dsi.h> +#include <linux/regmap.h> +#include <linux/module.h> + +#include "internal.h" + +static int dsi_reg_write(void *context, unsigned int reg, unsigned int val) +{ + struct device *dev = context; + struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev); + char payload[2]; + int ret; + + payload[0] = (char)reg; + payload[1] = (char)val; + + ret = mipi_dsi_generic_write(dsi, payload, 2); + return ret < 0 ? ret : 0; +} + +static int dsi_reg_read(void *context, unsigned int reg, unsigned int *val) +{ + struct device *dev = context; + struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev); + int ret; + + ret = mipi_dsi_generic_read(dsi, ®, 1, val, 1); + return ret < 0 ? ret : 0; +} + +static struct regmap_bus dsi_bus = { + .reg_write = dsi_reg_write, + .reg_read = dsi_reg_read, +}; + +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, + const struct regmap_config *config, + struct lock_class_key *lock_key, + const char *lock_name) +{ + return __regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, + lock_key, lock_name); +} +EXPORT_SYMBOL_GPL(__regmap_init_dsi); + +struct regmap *__devm_regmap_init_dsi(struct mipi_dsi_device *dsi, + const struct regmap_config *config, + struct lock_class_key *lock_key, + const char *lock_name) +{ + return __devm_regmap_init(&dsi->dev, &dsi_bus, &dsi->dev, config, + lock_key, lock_name); +} +EXPORT_SYMBOL_GPL(__devm_regmap_init_dsi); + +MODULE_LICENSE("GPL"); diff --git a/include/linux/regmap.h b/include/linux/regmap.h index dfe493ac692d..858239f7859f 100644 --- a/include/linux/regmap.h +++ b/include/linux/regmap.h @@ -32,6 +32,7 @@ struct regmap_range_cfg; struct regmap_field; struct snd_ac97; struct sdw_slave; +struct mipi_dsi_device; /* An enum of all the supported cache types */ enum regcache_type { @@ -573,6 +574,10 @@ struct regmap *__regmap_init_sdw(struct sdw_slave *sdw, const struct regmap_config *config, struct lock_class_key *lock_key, const char *lock_name); +struct regmap *__regmap_init_dsi(struct mipi_dsi_device *dsi, + const struct regmap_config *config, + struct lock_class_key *lock_key, + const char *lock_name); struct regmap *__devm_regmap_init(struct device *dev, const struct regmap_bus *bus, @@ -626,6 +631,10 @@ struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c, const struct regmap_config *config, struct lock_class_key *lock_key, const char *lock_name); +struct regmap *__devm_regmap_init_dsi(struct mipi_dsi_device *dsi, + const struct regmap_config *config, + struct lock_class_key *lock_key, + const char *lock_name); /* * Wrapper for regmap_init macros to include a unique lockdep key and name * for each call. No-op if CONFIG_LOCKDEP is not set. @@ -812,6 +821,19 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg); __regmap_lockdep_wrapper(__regmap_init_sdw, #config, \ sdw, config) +/** + * regmap_init_dsi() - Initialise register map + * + * @dsi: Device that will be interacted with + * @config: Configuration for register map + * + * The return value will be an ERR_PTR() on error or a valid pointer to + * a struct regmap. + */ +#define regmap_init_dsi(dsi, config) \ + __regmap_lockdep_wrapper(__regmap_init_dsi, #config, \ + dsi, config) + /** * devm_regmap_init() - Initialise managed register map @@ -999,6 +1021,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg); __regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config, \ i3c, config) +/** + * devm_regmap_init_dsi() - Initialise managed register map + * + * @dsi: Device that will be interacted with + * @config: Configuration for register map + * + * The return value will be an ERR_PTR() on error or a valid pointer + * to a struct regmap. The regmap will be automatically freed by the + * device management code. + */ +#define devm_regmap_init_dsi(dsi, config) \ + __regmap_lockdep_wrapper(__devm_regmap_init_dsi, #config, \ + dsi, config) + + int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk); void regmap_mmio_detach_clk(struct regmap *map); void regmap_exit(struct regmap *map);
Add basic support with a simple implementation that utilizes the generic read/write commands to allow device registers to be configured. Signed-off-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com> --- drivers/base/regmap/Kconfig | 6 +++- drivers/base/regmap/Makefile | 1 + drivers/base/regmap/regmap-dsi.c | 62 ++++++++++++++++++++++++++++++++ include/linux/regmap.h | 37 +++++++++++++++++++ 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 drivers/base/regmap/regmap-dsi.c