| Message ID | 1522871100-3621-2-git-send-email-austinwc@codeaurora.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Fast Mode Plus and other fixes | expand |
Hi Austin, On 4/5/2018 1:14 AM, Austin Christ wrote: > Add support for Qualcomm Centriq devices that are qup-v2 compatible but > do not support DMA, so nodma needs to be set. > > Signed-off-by: Austin Christ <austinwc@codeaurora.org> > --- > v1: > - Initial > v2: > - Remove whitespace error > --- > drivers/i2c/busses/i2c-qup.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c > index 904dfec..80af973 100644 > --- a/drivers/i2c/busses/i2c-qup.c > +++ b/drivers/i2c/busses/i2c-qup.c > @@ -1648,6 +1648,14 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup) > clk_disable_unprepare(qup->pclk); > } > > +#if IS_ENABLED(CONFIG_ACPI) > +static const struct acpi_device_id qup_i2c_acpi_match[] = { > + { "QCOM8010"}, > + { }, > +}; > +MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_match); > +#endif > + > static int qup_i2c_probe(struct platform_device *pdev) > { > static const int blk_sizes[] = {4, 16, 32}; > @@ -1675,6 +1683,12 @@ static int qup_i2c_probe(struct platform_device *pdev) > DEFAULT_CLK_FREQ); > } > > + if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) { > + qup->adap.algo = &qup_i2c_algo_v2; > + is_qup_v1 = false; > + goto nodma; > + } > + What happens without the above check. Does it not get resolved below, when qup_i2c_req_dma fails ? Regards, Sricharan
Hey Sricharan, On 4/11/2018 10:28 PM, Sricharan R wrote: > Hi Austin, > > On 4/5/2018 1:14 AM, Austin Christ wrote: >> Add support for Qualcomm Centriq devices that are qup-v2 compatible but >> do not support DMA, so nodma needs to be set. >> >> Signed-off-by: Austin Christ <austinwc@codeaurora.org> >> --- >> v1: >> - Initial >> v2: >> - Remove whitespace error >> --- >> drivers/i2c/busses/i2c-qup.c | 22 ++++++++++++++-------- >> 1 file changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c >> index 904dfec..80af973 100644 >> --- a/drivers/i2c/busses/i2c-qup.c >> +++ b/drivers/i2c/busses/i2c-qup.c >> @@ -1648,6 +1648,14 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup) >> clk_disable_unprepare(qup->pclk); >> } >> >> +#if IS_ENABLED(CONFIG_ACPI) >> +static const struct acpi_device_id qup_i2c_acpi_match[] = { >> + { "QCOM8010"}, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_match); >> +#endif >> + >> static int qup_i2c_probe(struct platform_device *pdev) >> { >> static const int blk_sizes[] = {4, 16, 32}; >> @@ -1675,6 +1683,12 @@ static int qup_i2c_probe(struct platform_device *pdev) >> DEFAULT_CLK_FREQ); >> } >> >> + if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) { >> + qup->adap.algo = &qup_i2c_algo_v2; >> + is_qup_v1 = false; >> + goto nodma; >> + } >> + > > What happens without the above check. Does it not get resolved below, > when qup_i2c_req_dma fails ? > > Regards, > Sricharan > Yes currently qup_i2c_req_dma fails and then the nodma: path is chosen. However, this causes unexpected logging of "tx channel not available" for the Centriq platform. This seemed a more correct solution to those incorrect logging statements than lowering the dev_err to dev_info or dev_notify.
Hi Austin, On 4/12/2018 8:57 PM, Christ, Austin wrote: > Hey Sricharan, > > On 4/11/2018 10:28 PM, Sricharan R wrote: >> Hi Austin, >> >> On 4/5/2018 1:14 AM, Austin Christ wrote: >>> Add support for Qualcomm Centriq devices that are qup-v2 compatible but >>> do not support DMA, so nodma needs to be set. >>> >>> Signed-off-by: Austin Christ <austinwc@codeaurora.org> >>> --- >>> v1: >>> - Initial >>> v2: >>> - Remove whitespace error >>> --- >>> drivers/i2c/busses/i2c-qup.c | 22 ++++++++++++++-------- >>> 1 file changed, 14 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c >>> index 904dfec..80af973 100644 >>> --- a/drivers/i2c/busses/i2c-qup.c >>> +++ b/drivers/i2c/busses/i2c-qup.c >>> @@ -1648,6 +1648,14 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup) >>> clk_disable_unprepare(qup->pclk); >>> } >>> +#if IS_ENABLED(CONFIG_ACPI) >>> +static const struct acpi_device_id qup_i2c_acpi_match[] = { >>> + { "QCOM8010"}, >>> + { }, >>> +}; >>> +MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_match); >>> +#endif >>> + >>> static int qup_i2c_probe(struct platform_device *pdev) >>> { >>> static const int blk_sizes[] = {4, 16, 32}; >>> @@ -1675,6 +1683,12 @@ static int qup_i2c_probe(struct platform_device *pdev) >>> DEFAULT_CLK_FREQ); >>> } >>> + if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) { >>> + qup->adap.algo = &qup_i2c_algo_v2; >>> + is_qup_v1 = false; >>> + goto nodma; >>> + } >>> + >> >> What happens without the above check. Does it not get resolved below, >> when qup_i2c_req_dma fails ? >> >> Regards, >> Sricharan >> > > Yes currently qup_i2c_req_dma fails and then the nodma: path is chosen. However, this causes unexpected logging of "tx channel not available" for the Centriq platform. This seemed a more correct solution to those incorrect logging statements than lowering the dev_err to dev_info or dev_notify. hmm, then just push this if (acpi_match_device(qup_i2c_acpi_match, qup->dev) down. if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) { .... } else { qup->adap.algo = &qup_i2c_algo_v2; is_qup_v1 = false; if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) goto nodma; else ret = qup_i2c_req_dma(qup); Regards, Sricharan
Hey Sricharan, On 4/13/2018 12:22 AM, Sricharan R wrote: > Hi Austin, > > On 4/12/2018 8:57 PM, Christ, Austin wrote: >> Hey Sricharan, >> >> On 4/11/2018 10:28 PM, Sricharan R wrote: >>> Hi Austin, >>> >>> On 4/5/2018 1:14 AM, Austin Christ wrote: >>>> Add support for Qualcomm Centriq devices that are qup-v2 compatible but >>>> do not support DMA, so nodma needs to be set. >>>> >>>> Signed-off-by: Austin Christ <austinwc@codeaurora.org> >>>> --- >>>> v1: >>>> - Initial >>>> v2: >>>> - Remove whitespace error >>>> --- >>>> drivers/i2c/busses/i2c-qup.c | 22 ++++++++++++++-------- >>>> 1 file changed, 14 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c >>>> index 904dfec..80af973 100644 >>>> --- a/drivers/i2c/busses/i2c-qup.c >>>> +++ b/drivers/i2c/busses/i2c-qup.c >>>> @@ -1648,6 +1648,14 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup) >>>> clk_disable_unprepare(qup->pclk); >>>> } >>>> +#if IS_ENABLED(CONFIG_ACPI) >>>> +static const struct acpi_device_id qup_i2c_acpi_match[] = { >>>> + { "QCOM8010"}, >>>> + { }, >>>> +}; >>>> +MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_match); >>>> +#endif >>>> + >>>> static int qup_i2c_probe(struct platform_device *pdev) >>>> { >>>> static const int blk_sizes[] = {4, 16, 32}; >>>> @@ -1675,6 +1683,12 @@ static int qup_i2c_probe(struct platform_device *pdev) >>>> DEFAULT_CLK_FREQ); >>>> } >>>> + if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) { >>>> + qup->adap.algo = &qup_i2c_algo_v2; >>>> + is_qup_v1 = false; >>>> + goto nodma; >>>> + } >>>> + >>> >>> What happens without the above check. Does it not get resolved below, >>> when qup_i2c_req_dma fails ? >>> >>> Regards, >>> Sricharan >>> >> >> Yes currently qup_i2c_req_dma fails and then the nodma: path is chosen. However, this causes unexpected logging of "tx channel not available" for the Centriq platform. This seemed a more correct solution to those incorrect logging statements than lowering the dev_err to dev_info or dev_notify. > > hmm, then just push this if (acpi_match_device(qup_i2c_acpi_match, qup->dev) down. > > if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) { > .... > } else { > qup->adap.algo = &qup_i2c_algo_v2; > is_qup_v1 = false; > if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) > goto nodma; > else > ret = qup_i2c_req_dma(qup); > Ok I'll make this change. > Regards, > Sricharan >
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 904dfec..80af973 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -1648,6 +1648,14 @@ static void qup_i2c_disable_clocks(struct qup_i2c_dev *qup) clk_disable_unprepare(qup->pclk); } +#if IS_ENABLED(CONFIG_ACPI) +static const struct acpi_device_id qup_i2c_acpi_match[] = { + { "QCOM8010"}, + { }, +}; +MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_match); +#endif + static int qup_i2c_probe(struct platform_device *pdev) { static const int blk_sizes[] = {4, 16, 32}; @@ -1675,6 +1683,12 @@ static int qup_i2c_probe(struct platform_device *pdev) DEFAULT_CLK_FREQ); } + if (acpi_match_device(qup_i2c_acpi_match, qup->dev)) { + qup->adap.algo = &qup_i2c_algo_v2; + is_qup_v1 = false; + goto nodma; + } + if (of_device_is_compatible(pdev->dev.of_node, "qcom,i2c-qup-v1.1.1")) { qup->adap.algo = &qup_i2c_algo; qup->adap.quirks = &qup_i2c_quirks; @@ -1959,14 +1973,6 @@ static int qup_i2c_resume(struct device *device) }; MODULE_DEVICE_TABLE(of, qup_i2c_dt_match); -#if IS_ENABLED(CONFIG_ACPI) -static const struct acpi_device_id qup_i2c_acpi_match[] = { - { "QCOM8010"}, - { }, -}; -MODULE_DEVICE_TABLE(acpi, qup_i2c_acpi_match); -#endif - static struct platform_driver qup_i2c_driver = { .probe = qup_i2c_probe, .remove = qup_i2c_remove,
Add support for Qualcomm Centriq devices that are qup-v2 compatible but do not support DMA, so nodma needs to be set. Signed-off-by: Austin Christ <austinwc@codeaurora.org> --- v1: - Initial v2: - Remove whitespace error --- drivers/i2c/busses/i2c-qup.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)