| Message ID | 20210928133143.157329-37-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | TI AM437X ADC1 | expand |
Hi Miquel,
I love your patch! Perhaps something to improve:
[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on jic23-iio/togreg robh/for-next v5.15-rc3 next-20210922]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/TI-AM437X-ADC1/20210928-213524
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: microblaze-randconfig-r033-20210928 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/43a01cf6413f2be038b0d466c7c3f6f16b40e2c3
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Miquel-Raynal/TI-AM437X-ADC1/20210928-213524
git checkout 43a01cf6413f2be038b0d466c7c3f6f16b40e2c3
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=microblaze
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/mfd/ti_am335x_tscadc.c: In function 'ti_tscadc_probe':
>> drivers/mfd/ti_am335x_tscadc.c:124:31: warning: variable 'use_mag' set but not used [-Wunused-but-set-variable]
124 | bool use_tsc = false, use_mag = false;
| ^~~~~~~
In file included from drivers/mfd/ti_am335x_tscadc.c:20:
drivers/mfd/ti_am335x_tscadc.c: At top level:
include/linux/mfd/ti_am335x_tscadc.h:126:38: error: 'HZ_PER_MHZ' undeclared here (not in a function)
126 | #define TSC_ADC_CLK (3 * HZ_PER_MHZ)
| ^~~~~~~~~~
drivers/mfd/ti_am335x_tscadc.c:354:28: note: in expansion of macro 'TSC_ADC_CLK'
354 | .target_clk_rate = TSC_ADC_CLK,
| ^~~~~~~~~~~
vim +/use_mag +124 drivers/mfd/ti_am335x_tscadc.c
114
115 static int ti_tscadc_probe(struct platform_device *pdev)
116 {
117 struct ti_tscadc_dev *tscadc;
118 struct resource *res;
119 struct clk *clk;
120 struct device_node *node;
121 struct mfd_cell *cell;
122 struct property *prop;
123 const __be32 *cur;
> 124 bool use_tsc = false, use_mag = false;
125 u32 val;
126 int err;
127 int tscmag_wires = 0, adc_channels = 0, cell_idx = 0, total_channels;
128 int readouts = 0, mag_tracks = 0;
129
130 /* Allocate memory for device */
131 tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL);
132 if (!tscadc)
133 return -ENOMEM;
134
135 tscadc->dev = &pdev->dev;
136
137 if (!pdev->dev.of_node) {
138 dev_err(&pdev->dev, "Could not find valid DT data.\n");
139 return -EINVAL;
140 }
141
142 tscadc->data = of_device_get_match_data(&pdev->dev);
143
144 if (ti_adc_with_touchscreen(tscadc)) {
145 node = of_get_child_by_name(pdev->dev.of_node, "tsc");
146 of_property_read_u32(node, "ti,wires", &tscmag_wires);
147 of_property_read_u32(node, "ti,coordiante-readouts", &readouts);
148 of_node_put(node);
149 if (tscmag_wires)
150 use_tsc = true;
151 } else {
152 /*
153 * When adding support for the magnetic stripe reader, here is
154 * the place to look for the number of tracks used from device
155 * tree. Let's default to 0 for now.
156 */
157 mag_tracks = 0;
158 tscmag_wires = mag_tracks * 2;
159 if (tscmag_wires)
160 use_mag = true;
161 }
162
163 node = of_get_child_by_name(pdev->dev.of_node, "adc");
164 of_property_for_each_u32(node, "ti,adc-channels", prop, cur, val) {
165 adc_channels++;
166 if (val > 7) {
167 dev_err(&pdev->dev, " PIN numbers are 0..7 (not %d)\n",
168 val);
169 of_node_put(node);
170 return -EINVAL;
171 }
172 }
173
174 of_node_put(node);
175
176 total_channels = tscmag_wires + adc_channels;
177 if (total_channels > 8) {
178 dev_err(&pdev->dev, "Number of i/p channels more than 8\n");
179 return -EINVAL;
180 }
181
182 if (total_channels == 0) {
183 dev_err(&pdev->dev, "Need atleast one channel.\n");
184 return -EINVAL;
185 }
186
187 if (use_tsc && (readouts * 2 + 2 + adc_channels > 16)) {
188 dev_err(&pdev->dev, "Too many step configurations requested\n");
189 return -EINVAL;
190 }
191
192 err = platform_get_irq(pdev, 0);
193 if (err < 0)
194 return err;
195 else
196 tscadc->irq = err;
197
198 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
199 tscadc->tscadc_base = devm_ioremap_resource(&pdev->dev, res);
200 if (IS_ERR(tscadc->tscadc_base))
201 return PTR_ERR(tscadc->tscadc_base);
202
203 tscadc->tscadc_phys_base = res->start;
204 tscadc->regmap = devm_regmap_init_mmio(&pdev->dev,
205 tscadc->tscadc_base,
206 &tscadc_regmap_config);
207 if (IS_ERR(tscadc->regmap)) {
208 dev_err(&pdev->dev, "regmap init failed\n");
209 return PTR_ERR(tscadc->regmap);
210 }
211
212 spin_lock_init(&tscadc->reg_lock);
213 init_waitqueue_head(&tscadc->reg_se_wait);
214
215 pm_runtime_enable(&pdev->dev);
216 pm_runtime_get_sync(&pdev->dev);
217
218 /*
219 * The TSC_ADC_Subsystem has 2 clock domains: OCP_CLK and ADC_CLK.
220 * ADCs produce a 12-bit sample every 15 ADC_CLK cycles.
221 * am33xx ADCs expect to capture 200ksps.
222 * am47xx ADCs expect to capture 867ksps.
223 * We need ADC clocks respectively running at 3MHz and 13MHz.
224 * These frequencies are valid since TSC_ADC_SS controller design
225 * assumes the OCP clock is at least 6x faster than the ADC clock.
226 */
227 clk = devm_clk_get(&pdev->dev, NULL);
228 if (IS_ERR(clk)) {
229 dev_err(&pdev->dev, "failed to get fck\n");
230 err = PTR_ERR(clk);
231 goto err_disable_clk;
232 }
233
234 tscadc->clk_div = (clk_get_rate(clk) / tscadc->data->target_clk_rate) - 1;
235 regmap_write(tscadc->regmap, REG_CLKDIV, tscadc->clk_div);
236
237 /*
238 * Set the control register bits. tscadc->ctrl stores the configuration
239 * of the CTRL register but not the subsystem enable bit which must be
240 * added manually when timely.
241 */
242 tscadc->ctrl = CNTRLREG_STEPID;
243 if (ti_adc_with_touchscreen(tscadc)) {
244 tscadc->ctrl |= CNTRLREG_TSC_STEPCONFIGWRT;
245 if (use_tsc) {
246 tscadc->ctrl |= CNTRLREG_TSC_ENB;
247 if (tscmag_wires == 5)
248 tscadc->ctrl |= CNTRLREG_TSC_5WIRE;
249 else
250 tscadc->ctrl |= CNTRLREG_TSC_4WIRE;
251 }
252 } else {
253 tscadc->ctrl |= CNTRLREG_MAG_PREAMP_PWRDOWN |
254 CNTRLREG_MAG_PREAMP_BYPASS;
255 }
256 regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl);
257
258 tscadc_idle_config(tscadc);
259
260 /* Enable the TSC module enable bit */
261 regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl | CNTRLREG_SSENB);
262
263 /* TSC or MAG Cell */
264 if (tscmag_wires > 0) {
265 cell = &tscadc->cells[cell_idx++];
266 cell->name = tscadc->data->secondary_feature_name;
267 cell->of_compatible = tscadc->data->secondary_feature_compatible;
268 cell->platform_data = &tscadc;
269 cell->pdata_size = sizeof(tscadc);
270 }
271
272 /* ADC Cell */
273 if (adc_channels > 0) {
274 cell = &tscadc->cells[cell_idx++];
275 cell->name = tscadc->data->adc_feature_name;
276 cell->of_compatible = tscadc->data->adc_feature_compatible;
277 cell->platform_data = &tscadc;
278 cell->pdata_size = sizeof(tscadc);
279 }
280
281 err = mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
282 tscadc->cells, cell_idx, NULL, 0, NULL);
283 if (err < 0)
284 goto err_disable_clk;
285
286 platform_set_drvdata(pdev, tscadc);
287 return 0;
288
289 err_disable_clk:
290 pm_runtime_put_sync(&pdev->dev);
291 pm_runtime_disable(&pdev->dev);
292
293 return err;
294 }
295
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hello Lee, - reduced a bit the Cc: list lkp@intel.com wrote on Wed, 29 Sep 2021 06:32:16 +0800: > Hi Miquel, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on lee-mfd/for-mfd-next] > [also build test WARNING on jic23-iio/togreg robh/for-next v5.15-rc3 next-20210922] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use '--base' as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/TI-AM437X-ADC1/20210928-213524 > base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next > config: microblaze-randconfig-r033-20210928 (attached as .config) > compiler: microblaze-linux-gcc (GCC) 11.2.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/0day-ci/linux/commit/43a01cf6413f2be038b0d466c7c3f6f16b40e2c3 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Miquel-Raynal/TI-AM437X-ADC1/20210928-213524 > git checkout 43a01cf6413f2be038b0d466c7c3f6f16b40e2c3 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=microblaze > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > drivers/mfd/ti_am335x_tscadc.c: In function 'ti_tscadc_probe': > >> drivers/mfd/ti_am335x_tscadc.c:124:31: warning: variable 'use_mag' set but not used [-Wunused-but-set-variable] > 124 | bool use_tsc = false, use_mag = false; > | ^~~~~~~ It's strange, on my side I didn't get any warnings (using an older GCC, perhaps not with all the flags enabled either). Anyway, let's remove the noise, I have a fixup patch which can be applied without disturbing any of the other patches. Lee, do you prefer that I send a fixup patch or a v5 for patch "mfd: ti_am335x_tscadc: Add ADC1/magnetic reader support"? (provided of course that the other patches are fine to you). > In file included from drivers/mfd/ti_am335x_tscadc.c:20: > drivers/mfd/ti_am335x_tscadc.c: At top level: > include/linux/mfd/ti_am335x_tscadc.h:126:38: error: 'HZ_PER_MHZ' undeclared here (not in a function) > 126 | #define TSC_ADC_CLK (3 * HZ_PER_MHZ) > | ^~~~~~~~~~ > drivers/mfd/ti_am335x_tscadc.c:354:28: note: in expansion of macro 'TSC_ADC_CLK' > 354 | .target_clk_rate = TSC_ADC_CLK, > | ^~~~~~~~~~~ For this one I don't believe this is a real warning, units.h is introduced in a precedent patch, HZ_PER_MHZ is a new macro but it is upstream now so I think it can safely be discarded. Thanks, Miquèl
On Fri, 01 Oct 2021, Miquel Raynal wrote: > Hello Lee, > > - reduced a bit the Cc: list > > lkp@intel.com wrote on Wed, 29 Sep 2021 06:32:16 +0800: > > > Hi Miquel, > > > > I love your patch! Perhaps something to improve: > > > > [auto build test WARNING on lee-mfd/for-mfd-next] > > [also build test WARNING on jic23-iio/togreg robh/for-next v5.15-rc3 next-20210922] > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > And when submitting patch, we suggest to use '--base' as documented in > > https://git-scm.com/docs/git-format-patch] > > > > url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/TI-AM437X-ADC1/20210928-213524 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next > > config: microblaze-randconfig-r033-20210928 (attached as .config) > > compiler: microblaze-linux-gcc (GCC) 11.2.0 > > reproduce (this is a W=1 build): > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # https://github.com/0day-ci/linux/commit/43a01cf6413f2be038b0d466c7c3f6f16b40e2c3 > > git remote add linux-review https://github.com/0day-ci/linux > > git fetch --no-tags linux-review Miquel-Raynal/TI-AM437X-ADC1/20210928-213524 > > git checkout 43a01cf6413f2be038b0d466c7c3f6f16b40e2c3 > > # save the attached .config to linux build tree > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=microblaze > > > > If you fix the issue, kindly add following tag as appropriate > > Reported-by: kernel test robot <lkp@intel.com> > > > > All warnings (new ones prefixed by >>): > > > > drivers/mfd/ti_am335x_tscadc.c: In function 'ti_tscadc_probe': > > >> drivers/mfd/ti_am335x_tscadc.c:124:31: warning: variable 'use_mag' set but not used [-Wunused-but-set-variable] > > 124 | bool use_tsc = false, use_mag = false; > > | ^~~~~~~ > > It's strange, on my side I didn't get any warnings (using an older GCC, > perhaps not with all the flags enabled either). Anyway, let's remove > the noise, I have a fixup patch which can be applied without disturbing > any of the other patches. > > Lee, do you prefer that I send a fixup patch or a v5 for patch "mfd: > ti_am335x_tscadc: Add ADC1/magnetic reader support"? (provided of > course that the other patches are fine to you). Fix this patch up please. It would be odd to apply a known broken patch. > > In file included from drivers/mfd/ti_am335x_tscadc.c:20: > > drivers/mfd/ti_am335x_tscadc.c: At top level: > > include/linux/mfd/ti_am335x_tscadc.h:126:38: error: 'HZ_PER_MHZ' undeclared here (not in a function) > > 126 | #define TSC_ADC_CLK (3 * HZ_PER_MHZ) > > | ^~~~~~~~~~ > > drivers/mfd/ti_am335x_tscadc.c:354:28: note: in expansion of macro 'TSC_ADC_CLK' > > 354 | .target_clk_rate = TSC_ADC_CLK, > > | ^~~~~~~~~~~ > > For this one I don't believe this is a real warning, units.h is > introduced in a precedent patch, HZ_PER_MHZ is a new macro but it is > upstream now so I think it can safely be discarded. > > Thanks, > Miquèl
diff --git a/drivers/mfd/ti_am335x_tscadc.c b/drivers/mfd/ti_am335x_tscadc.c index 4f76b5498077..6d852ce3be51 100644 --- a/drivers/mfd/ti_am335x_tscadc.c +++ b/drivers/mfd/ti_am335x_tscadc.c @@ -121,11 +121,11 @@ static int ti_tscadc_probe(struct platform_device *pdev) struct mfd_cell *cell; struct property *prop; const __be32 *cur; - bool use_tsc = false; + bool use_tsc = false, use_mag = false; u32 val; int err; int tscmag_wires = 0, adc_channels = 0, cell_idx = 0, total_channels; - int readouts = 0; + int readouts = 0, mag_tracks = 0; /* Allocate memory for device */ tscadc = devm_kzalloc(&pdev->dev, sizeof(*tscadc), GFP_KERNEL); @@ -148,6 +148,16 @@ static int ti_tscadc_probe(struct platform_device *pdev) of_node_put(node); if (tscmag_wires) use_tsc = true; + } else { + /* + * When adding support for the magnetic stripe reader, here is + * the place to look for the number of tracks used from device + * tree. Let's default to 0 for now. + */ + mag_tracks = 0; + tscmag_wires = mag_tracks * 2; + if (tscmag_wires) + use_mag = true; } node = of_get_child_by_name(pdev->dev.of_node, "adc"); @@ -209,8 +219,9 @@ static int ti_tscadc_probe(struct platform_device *pdev) * The TSC_ADC_Subsystem has 2 clock domains: OCP_CLK and ADC_CLK. * ADCs produce a 12-bit sample every 15 ADC_CLK cycles. * am33xx ADCs expect to capture 200ksps. - * We need the ADC clocks to run at 3MHz. - * This frequency is valid since TSC_ADC_SS controller design + * am47xx ADCs expect to capture 867ksps. + * We need ADC clocks respectively running at 3MHz and 13MHz. + * These frequencies are valid since TSC_ADC_SS controller design * assumes the OCP clock is at least 6x faster than the ADC clock. */ clk = devm_clk_get(&pdev->dev, NULL); @@ -238,6 +249,9 @@ static int ti_tscadc_probe(struct platform_device *pdev) else tscadc->ctrl |= CNTRLREG_TSC_4WIRE; } + } else { + tscadc->ctrl |= CNTRLREG_MAG_PREAMP_PWRDOWN | + CNTRLREG_MAG_PREAMP_BYPASS; } regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl); @@ -246,7 +260,7 @@ static int ti_tscadc_probe(struct platform_device *pdev) /* Enable the TSC module enable bit */ regmap_write(tscadc->regmap, REG_CTRL, tscadc->ctrl | CNTRLREG_SSENB); - /* TSC Cell */ + /* TSC or MAG Cell */ if (tscmag_wires > 0) { cell = &tscadc->cells[cell_idx++]; cell->name = tscadc->data->secondary_feature_name; @@ -340,8 +354,17 @@ static const struct ti_tscadc_data tscdata = { .target_clk_rate = TSC_ADC_CLK, }; +static const struct ti_tscadc_data magdata = { + .adc_feature_name = "TI-am43xx-adc", + .adc_feature_compatible = "ti,am4372-adc", + .secondary_feature_name = "TI-am43xx-mag", + .secondary_feature_compatible = "ti,am4372-mag", + .target_clk_rate = MAG_ADC_CLK, +}; + static const struct of_device_id ti_tscadc_dt_ids[] = { { .compatible = "ti,am3359-tscadc", .data = &tscdata }, + { .compatible = "ti,am4372-magadc", .data = &magdata }, { } }; MODULE_DEVICE_TABLE(of, ti_tscadc_dt_ids); @@ -359,6 +382,6 @@ static struct platform_driver ti_tscadc_driver = { module_platform_driver(ti_tscadc_driver); -MODULE_DESCRIPTION("TI touchscreen / ADC MFD controller driver"); +MODULE_DESCRIPTION("TI touchscreen/magnetic stripe reader/ADC MFD controller driver"); MODULE_AUTHOR("Rachna Patil <rachna@ti.com>"); MODULE_LICENSE("GPL"); diff --git a/include/linux/mfd/ti_am335x_tscadc.h b/include/linux/mfd/ti_am335x_tscadc.h index ee160b2036c1..5225e3fc194d 100644 --- a/include/linux/mfd/ti_am335x_tscadc.h +++ b/include/linux/mfd/ti_am335x_tscadc.h @@ -106,6 +106,11 @@ #define CNTRLREG_TSC_8WIRE CNTRLREG_TSC_AFE_CTRL(3) #define CNTRLREG_TSC_ENB BIT(7) +/*Control registers bitfields for MAGADC IP */ +#define CNTRLREG_MAGADCENB BIT(0) +#define CNTRLREG_MAG_PREAMP_PWRDOWN BIT(5) +#define CNTRLREG_MAG_PREAMP_BYPASS BIT(6) + /* FIFO READ Register */ #define FIFOREAD_DATA_MASK GENMASK(11, 0) #define FIFOREAD_CHNLID_MASK GENMASK(19, 16) @@ -119,6 +124,7 @@ #define CHARGE_STEP 0x11 #define TSC_ADC_CLK (3 * HZ_PER_MHZ) +#define MAG_ADC_CLK (13 * HZ_PER_MHZ) #define TOTAL_STEPS 16 #define TOTAL_CHANNELS 8 #define FIFO1_THRESHOLD 19