> +static int rzn1_adc_read_raw_ch(struct rzn1_adc *rzn1_adc, unsigned int chan, int *val) > +{ > + u32 *adc1_data, *adc2_data; > + int adc1_ch, adc2_ch; > + u32 adc_data; > + int ret; > + > + if (chan < 8) { > + /* chan 0..7 used to get ADC1 ch 0..7 */ > + adc1_ch = chan; > + adc1_data = &adc_data; > + adc2_ch = -1; > + adc2_data = NULL; > + } else if (chan < 16) { > + /* chan 8..15 used to get ADC2 ch 0..7 */ > + adc1_ch = -1; > + adc1_data = NULL; > + adc2_ch = chan - 8; > + adc2_data = &adc_data; > + } else { > + return -EINVAL; > + } How about putting part of the logic into the setup function? So, here only: if (chan >= 16) return -EINVAL > + > + ret = pm_runtime_resume_and_get(rzn1_adc->dev); > + if (ret < 0) > + return ret; > + > + mutex_lock(&rzn1_adc->lock); > + > + rzn1_adc_vc_setup_conversion(rzn1_adc, chan, adc1_ch, adc2_ch); rzn1_adc_vc_setup_conversion(rzn1_adc, chan); And in that function: > +static void rzn1_adc_vc_setup_conversion(struct rzn1_adc *rzn1_adc, u32 ch, > + int adc1_ch, int adc2_ch) > +{ > + u32 vc = 0; > + > + if (adc1_ch != -1) > + vc |= RZN1_ADC_VC_ADC1_ENABLE | RZN1_ADC_VC_ADC1_CHANNEL_SEL(adc1_ch); > + > + if (adc2_ch != -1) > + vc |= RZN1_ADC_VC_ADC2_ENABLE | RZN1_ADC_VC_ADC2_CHANNEL_SEL(adc2_ch); > + > + writel(vc, rzn1_adc->regs + RZN1_ADC_VC_REG(ch)); > +} if (ch < 8) vc |= RZN1_ADC_VC_ADC1_ENABLE | RZN1_ADC_VC_ADC1_CHANNEL_SEL(ch); else vc |= RZN1_ADC_VC_ADC2_ENABLE | RZN1_ADC_VC_ADC2_CHANNEL_SEL(ch - 8); And a similar simplification for rzn1_adc_vc_wait_conversion(). Should work and the code is even more readable, I'd say. And has less lines.