On Tue, 24 Oct 2017, Baolin Wang wrote:
> Hi Lee,
>
> On 24 October 2017 at 17:26, Lee Jones <lee.jones@xxxxxxxxxx> wrote:
> > On Mon, 16 Oct 2017, Baolin Wang wrote:
> >
> >> This patch adds support for Spreadtrum SC27xx series PMIC MFD core, and It
> >> provides communication through the SPI interfaces. The SC27xx series PMICs
> >> contains the following 6 major components:
> >> - DCDCs
> >> - LDOs
> >> - Battery management system
> >> - Audio codec
> >> - User interface function, such as indicator, flash LED
> >> - IC level function, such as power on/off, type-c
> >>
> >> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxx>
> >> ---
> >> Changes since v1:
> >> - Re-order the including head files.
> >> - Add one condition to check register size and value size when reading.
> >> - Fix some coding style issues.
> >> ---
> >> drivers/mfd/Kconfig | 10 ++
> >> drivers/mfd/Makefile | 1 +
> >> drivers/mfd/sprd-sc27xx-spi.c | 264 +++++++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 275 insertions(+)
> >> create mode 100644 drivers/mfd/sprd-sc27xx-spi.c
[...]
> >> +static int sprd_pmic_spi_read(void *context,
> >> + const void *reg, size_t reg_size,
> >> + void *val, size_t val_size)
> >> +{
> >> + struct device *dev = context;
> >> + struct spi_device *spi = to_spi_device(dev);
> >> + u32 rx_buf[2] = { 0 };
> >> + int ret;
> >> +
> >> + /* Now we only support one PMIC register to read every time. */
> >> + if (reg_size != sizeof(u32) || val_size != sizeof(u32))
> >> + return -EINVAL;
> >> +
> >> + rx_buf[0] = *(u32 *)reg;
> >
> > This looks dodgy. You should not be doing raw memory manipulation.
> >
> > You need to be passing __iomem and using one of the predefined APIs
> > (i.e. readl) on it.
>
> Sorry, I did not get you here. The 'reg' parameter is just one PMIC
> register offset which want to be read. We do not need to pass __iomem
> to SPI master driver, we just pass one register offset address to our
> SPI master driver then the SPI master driver will handle to read
> values from our PMIC.
You should not be referencing the value at 'reg' manually:
rx_buf[0] = readl(reg);
[...]
> >> +static struct spi_driver sprd_pmic_driver = {
> >> + .driver = {
> >> + .name = "sc27xx-pmic",
> >> + .bus = &spi_bus_type,
> >> + .of_match_table = sprd_pmic_match,
> >
> > of_match_ptr()?
>
> The of_match_ptr() did nothing, so I think it is not necessary.
What do you mean?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
